Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix register-docs feature not being tested #942

Merged
merged 5 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions .github/workflows/full-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ on:


env:
GDEXT_FEATURES: ''
# Applies to all 'register-docs' features across crates.
CLIPPY_FEATURES: '--features register-docs'
TEST_FEATURES: ''
RETRY: ${{ github.workspace }}/.github/other/retry.sh

# ASan options: https://github.com/google/sanitizers/wiki/AddressSanitizerFlags
Expand Down Expand Up @@ -99,7 +101,7 @@ jobs:
# Note: could use `-- --no-deps` to not lint dependencies, however it doesn't really speed up and also skips deps in workspace.
- name: "Check clippy"
run: |
cargo clippy --all-targets $GDEXT_FEATURES -- \
cargo clippy --all-targets $CLIPPY_FEATURES -- \
-D clippy::suspicious \
-D clippy::style \
-D clippy::complexity \
Expand Down Expand Up @@ -161,10 +163,10 @@ jobs:
run: cargo +nightly update -Z minimal-versions

- name: "Compile tests"
run: cargo test $GDEXT_FEATURES --no-run ${{ matrix.rust-extra-args }}
run: cargo test $TEST_FEATURES --no-run ${{ matrix.rust-extra-args }}

- name: "Test"
run: cargo test $GDEXT_FEATURES ${{ matrix.rust-extra-args }}
run: cargo test $TEST_FEATURES ${{ matrix.rust-extra-args }}


miri-test:
Expand Down
11 changes: 6 additions & 5 deletions .github/workflows/minimal-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ on:


env:
GDEXT_FEATURES: ''
# GDEXT_FEATURES: '--features crate/feature'
# Applies to all 'register-docs' features across crates.
CLIPPY_FEATURES: '--features register-docs'
TEST_FEATURES: ''
# GDEXT_CRATE_ARGS: '-p godot-codegen -p godot-ffi -p godot-core -p godot-macros -p godot'
RETRY: ${{ github.workspace }}/.github/other/retry.sh

Expand Down Expand Up @@ -96,7 +97,7 @@ jobs:

- name: "Check clippy"
run: |
cargo clippy --all-targets $GDEXT_FEATURES -- \
cargo clippy --all-targets $CLIPPY_FEATURES -- \
-D clippy::suspicious \
-D clippy::style \
-D clippy::complexity \
Expand All @@ -120,10 +121,10 @@ jobs:
uses: ./.github/composite/rust

- name: "Compile tests"
run: cargo test $GDEXT_FEATURES --no-run
run: cargo test $TEST_FEATURES --no-run

- name: "Test"
run: cargo test $GDEXT_FEATURES
run: cargo test $TEST_FEATURES


# For complex matrix workflow, see https://stackoverflow.com/a/65434401
Expand Down
2 changes: 1 addition & 1 deletion godot-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ homepage = "https://godot-rust.github.io"

[features]
default = []
docs = []
register-docs = []
codegen-rustfmt = ["godot-ffi/codegen-rustfmt", "godot-codegen/codegen-rustfmt"]
codegen-full = ["godot-codegen/codegen-full"]
codegen-lazy-fptrs = [
Expand Down
1 change: 1 addition & 0 deletions godot-core/src/docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ pub fn gather_xml_docs() -> impl Iterator<Item = String> {
PluginItem::Struct {
docs: Some(docs), ..
} => map.entry(class_name).or_default().definition = docs,

_ => (),
}
});
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/init/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ unsafe fn gdext_on_level_init(level: InitLevel) {
unsafe { ensure_godot_features_compatible() };
}
InitLevel::Editor => {
#[cfg(all(since_api = "4.3", feature = "docs"))]
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
// SAFETY: Godot binding is initialized, and this is called from the main thread.
unsafe {
crate::docs::register();
Expand Down
6 changes: 3 additions & 3 deletions godot-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
pub mod builder;
pub mod builtin;
pub mod classes;
#[cfg(all(since_api = "4.3", feature = "docs"))]
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
pub mod docs;
#[doc(hidden)]
pub mod possibly_docs {
#[cfg(all(since_api = "4.3", feature = "docs"))]
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
pub use crate::docs::*;
}
pub mod global;
Expand All @@ -35,7 +35,7 @@ pub use godot_ffi as sys;
// ----------------------------------------------------------------------------------------------------------------------------------------------
// Validations (see also godot/lib.rs)

#[cfg(all(feature = "docs", before_api = "4.3"))]
#[cfg(all(feature = "register-docs", before_api = "4.3"))]
compile_error!("Generating editor docs for Rust symbols requires at least Godot 4.3.");

// ----------------------------------------------------------------------------------------------------------------------------------------------
Expand Down
6 changes: 3 additions & 3 deletions godot-core/src/registry/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) {
is_editor_plugin,
is_internal,
is_instantiable,
#[cfg(all(since_api = "4.3", feature = "docs"))]
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
docs: _,
} => {
c.parent_class_name = Some(base_class_name);
Expand Down Expand Up @@ -296,7 +296,7 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) {
PluginItem::InherentImpl(InherentImpl {
register_methods_constants_fn,
register_rpcs_fn: _,
#[cfg(all(since_api = "4.3", feature = "docs"))]
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
docs: _,
}) => {
c.register_methods_constants_fn = Some(register_methods_constants_fn);
Expand All @@ -315,7 +315,7 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) {
user_free_property_list_fn,
user_property_can_revert_fn,
user_property_get_revert_fn,
#[cfg(all(since_api = "4.3", feature = "docs"))]
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
virtual_method_docs: _,
} => {
c.user_register_fn = user_register_fn;
Expand Down
8 changes: 4 additions & 4 deletions godot-core/src/registry/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/

#[cfg(all(since_api = "4.3", feature = "docs"))]
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
use crate::docs::*;
use crate::init::InitLevel;
use crate::meta::ClassName;
Expand Down Expand Up @@ -65,7 +65,7 @@ pub struct InherentImpl {
///
/// This function is called in [`UserClass::__before_ready()`](crate::obj::UserClass::__before_ready) definitions generated by the `#[derive(GodotClass)]` macro.
pub register_rpcs_fn: Option<ErasedRegisterRpcsFn>,
#[cfg(all(since_api = "4.3", feature = "docs"))]
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
pub docs: InherentImplDocs,
}

Expand Down Expand Up @@ -121,7 +121,7 @@ pub enum PluginItem {

/// Whether the class has a default constructor.
is_instantiable: bool,
#[cfg(all(since_api = "4.3", feature = "docs"))]
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
docs: Option<StructDocs>,
},

Expand All @@ -130,7 +130,7 @@ pub enum PluginItem {

/// Collected from `#[godot_api] impl I... for MyClass`.
ITraitImpl {
#[cfg(all(since_api = "4.3", feature = "docs"))]
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
/// Virtual method documentation.
virtual_method_docs: &'static str,
/// Callback to user-defined `register_class` function.
Expand Down
2 changes: 1 addition & 1 deletion godot-macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ homepage = "https://godot-rust.github.io"

[features]
api-custom = ["godot-bindings/api-custom"]
docs = ["dep:markdown"]
register-docs = ["dep:markdown"]
codegen-full = ["godot/__codegen-full"]

[lib]
Expand Down
4 changes: 2 additions & 2 deletions godot-macros/src/class/data_models/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub struct Field {
pub var: Option<FieldVar>,
pub export: Option<FieldExport>,
pub is_onready: bool,
#[cfg(feature = "docs")]
#[cfg(feature = "register-docs")]
pub attributes: Vec<venial::Attribute>,
}

Expand All @@ -28,7 +28,7 @@ impl Field {
var: None,
export: None,
is_onready: false,
#[cfg(feature = "docs")]
#[cfg(feature = "register-docs")]
attributes: field.attributes.clone(),
}
}
Expand Down
4 changes: 2 additions & 2 deletions godot-macros/src/class/data_models/inherent_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ pub fn transform_inherent_impl(mut impl_block: venial::Impl) -> ParseResult<Toke
let (funcs, signals) = process_godot_fns(&class_name, &mut impl_block)?;
let consts = process_godot_constants(&mut impl_block)?;

#[cfg(all(feature = "docs", since_api = "4.3"))]
#[cfg(all(feature = "register-docs", since_api = "4.3"))]
let docs = crate::docs::make_inherent_impl_docs(&funcs, &consts, &signals);
#[cfg(not(all(feature = "docs", since_api = "4.3")))]
#[cfg(not(all(feature = "register-docs", since_api = "4.3")))]
let docs = quote! {};

let signal_registrations = make_signal_registrations(signals, &class_name_obj);
Expand Down
4 changes: 2 additions & 2 deletions godot-macros/src/class/data_models/interface_trait_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ pub fn transform_trait_impl(original_impl: venial::Impl) -> ParseResult<TokenStr
let mut virtual_method_names = vec![];

let prv = quote! { ::godot::private };
#[cfg(all(feature = "docs", since_api = "4.3"))]
#[cfg(all(feature = "register-docs", since_api = "4.3"))]
let docs = crate::docs::make_virtual_impl_docs(&original_impl.body_items);
#[cfg(not(all(feature = "docs", since_api = "4.3")))]
#[cfg(not(all(feature = "register-docs", since_api = "4.3")))]
let docs = quote! {};
for item in original_impl.body_items.iter() {
let method = if let venial::ImplMember::AssocFunction(f) = item {
Expand Down
4 changes: 2 additions & 2 deletions godot-macros/src/class/derive_godot_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult<TokenStream> {

let is_internal = struct_cfg.is_internal;
let base_ty = &struct_cfg.base_ty;
#[cfg(all(feature = "docs", since_api = "4.3"))]
#[cfg(all(feature = "register-docs", since_api = "4.3"))]
let docs = crate::docs::make_definition_docs(
base_ty.to_string(),
&class.attributes,
&fields.all_fields,
);
#[cfg(not(all(feature = "docs", since_api = "4.3")))]
#[cfg(not(all(feature = "register-docs", since_api = "4.3")))]
let docs = quote! {};
let base_class = quote! { ::godot::classes::#base_ty };
let base_class_name_obj = util::class_name_obj(&base_class);
Expand Down
6 changes: 3 additions & 3 deletions godot-macros/src/docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub fn make_definition_docs(
let base_escaped = xml_escape(base);
let desc_escaped = xml_escape(make_docs_from_attributes(description)?);
let members = members
.into_iter()
.iter()
.filter(|x| x.var.is_some() | x.export.is_some())
.filter_map(member)
.collect::<String>();
Expand Down Expand Up @@ -112,7 +112,7 @@ fn siphon_docs_from_attributes(doc: &[Attribute]) -> impl Iterator<Item = String
_ => None,
})
.flat_map(|doc| {
doc.into_iter().map(|x| {
doc.iter().map(|x| {
x.to_string()
.trim_start_matches('r')
.trim_start_matches('#')
Expand All @@ -126,7 +126,7 @@ fn siphon_docs_from_attributes(doc: &[Attribute]) -> impl Iterator<Item = String

fn xml_escape(value: String) -> String {
// Most strings have no special characters, so this check helps avoid unnecessary string copying
if !value.contains(&['&', '<', '>', '"', '\'']) {
if !value.contains(['&', '<', '>', '"', '\'']) {
return value;
}

Expand Down
2 changes: 1 addition & 1 deletion godot-macros/src/docs/markdown_converter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ fn walk_node(node: &md::Node, definitions: &HashMap<&str, &str>) -> Option<Strin

Html(html) => html.value.clone(),

_ => walk_nodes(&node.children()?, definitions, ""),
_ => walk_nodes(node.children()?, definitions, ""),
};

Some(bbcode)
Expand Down
2 changes: 1 addition & 1 deletion godot-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
mod bench;
mod class;
mod derive;
#[cfg(all(feature = "docs", since_api = "4.3"))]
#[cfg(all(feature = "register-docs", since_api = "4.3"))]
mod docs;
mod gdextension;
mod itest;
Expand Down
2 changes: 1 addition & 1 deletion godot/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ codegen-rustfmt = ["godot-core/codegen-rustfmt"]
lazy-function-tables = ["godot-core/codegen-lazy-fptrs"]
serde = ["godot-core/serde"]

register-docs = ["godot-macros/docs", "godot-core/docs"]
register-docs = ["godot-macros/register-docs", "godot-core/register-docs"]

api-custom = ["godot-core/api-custom"]
# [version-sync] [[
Expand Down
2 changes: 1 addition & 1 deletion itest/rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ default = ["codegen-full"]
codegen-full = ["godot/__codegen-full"]
codegen-full-experimental = ["codegen-full", "godot/experimental-godot-api"]
experimental-threads = ["godot/experimental-threads"]
register-docs = ["godot/register-docs"] # TODO remove as soon as constant_test.rs checks bitfields with #[constant] proc-macro
register-docs = ["godot/register-docs"]
serde = ["dep:serde", "dep:serde_json", "godot/serde"]

# Do not add features here that are 1:1 forwarded to the `godot` crate, unless they are needed by itest itself.
Expand Down
4 changes: 2 additions & 2 deletions itest/rust/src/register_tests/constant_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ impl godot::obj::cap::ImplementsGodotApi for HasOtherConstants {
}
}

// TODO once this is done via proc-macro, remove `register-docs` feature from itest, and update CI workflows.
// TODO once this is done via proc-macro, see if `register-docs` is still used in register_docs_test.rs. Otherwise, remove feature from Cargo.toml.
godot::sys::plugin_add!(
__GODOT_PLUGIN_REGISTRY in ::godot::private;
::godot::private::ClassPlugin {
Expand All @@ -174,7 +174,7 @@ godot::sys::plugin_add!(
raw: ::godot::private::callbacks::register_user_methods_constants::<HasOtherConstants>,
},
register_rpcs_fn: None,
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
#[cfg(feature = "register-docs")]
docs: ::godot::docs::InherentImplDocs::default(),
}),
init_level: HasOtherConstants::INIT_LEVEL,
Expand Down
1 change: 1 addition & 0 deletions itest/rust/src/register_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ mod func_test;
mod gdscript_ffi_test;
mod naming_tests;
mod option_ffi_test;
mod register_docs_test;
#[cfg(feature = "codegen-full")]
mod rpc_test;
mod var_test;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
#![cfg(feature = "register-docs")]
/*
* Copyright (c) godot-rust; Bromeon and contributors.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/

#![cfg(feature = "register-docs")]

use crate::framework::itest;
use godot::prelude::*;

/// *documented* ~ **documented** ~ [AABB] < [pr](https://github.com/godot-rust/gdext/pull/748)
Expand Down Expand Up @@ -187,15 +190,25 @@ impl FairlyDocumented {
fn documented_signal(p: Vector3, w: f64, node: Gd<Node>);
}

#[test]
fn correct() {
#[itest]
fn test_register_docs() {
let xml = find_class_docs("FairlyDocumented");

// Uncomment if implementation changes and expected output file should be rewritten.
// std::fs::write(
// "tests/test_data/docs.xml",
// godot_core::docs::gather_xml_docs().next().unwrap(),
// );
assert_eq!(
include_str!("test_data/docs.xml"),
godot_core::docs::gather_xml_docs().next().unwrap()
);
// std::fs::write("../rust/src/register_tests/res/registered_docs.xml", &xml)
// .expect("failed to write docs XML file");

assert_eq!(include_str!("res/registered_docs.xml"), xml);
}

fn find_class_docs(class_name: &str) -> String {
let mut count = 0;
for xml in godot::docs::gather_xml_docs() {
count += 1;
if xml.contains(class_name) {
return xml;
}
}

panic!("Registered docs for class {class_name} not found in {count} XML files");
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ these</description>
</description>
</method>

<method name="virtual_documented">
<method name="_virtual_documented">
<return type="()" />
<param index="0" name="node" type="Gd &lt; Node &gt;" />
<description>
Expand Down
Loading