Broken ignored doctest in subxt macro output.

[deepink-mas]

I have this code

#[subxt::subxt(runtime_metadata_path = "metadata.scale")]
pub mod node_runtime {}

which expands into almost 7k lines of code, among which is this doctest which does not even compile

		pub mod constants {
			use super::runtime_types;
			pub struct ConstantsApi;

			impl ConstantsApi {
				#[doc = " A fee mulitplier for `Operational` extrinsics to compute \"virtual tip\" to boost their"]
				#[doc = " `priority`"]
				#[doc = ""]
				#[doc = " This value is multipled by the `final_fee` to obtain a \"virtual tip\" that is later"]
				#[doc = " added to a tip component in regular `priority` calculations."]
				#[doc = " It means that a `Normal` transaction can front-run a similarly-sized `Operational`"]
				#[doc = " extrinsic (with no tip), by including a tip value greater than the virtual tip."]
				#[doc = ""]
				#[doc = " ```rust,ignore"]
				#[doc = " // For `Normal`"]
				#[doc = " let priority = priority_calc(tip);"]
				#[doc = ""]
				#[doc = " // For `Operational`"]
				#[doc = " let virtual_tip = (inclusion_fee + tip) * OperationalFeeMultiplier;"]
				#[doc = " let priority = priority_calc(tip + virtual_tip);"]
				#[doc = " ```"]
				#[doc = ""]
				#[doc = " Note that since we use `final_fee` the multiplier applies also to the regular `tip`"]
				#[doc = " sent with the transaction. So, not only does the transaction get a priority bump based"]
				#[doc = " on the `inclusion_fee`, but we also amplify the impact of tips applied to `Operational`"]
				#[doc = " transactions."]
				pub fn operational_fee_multiplier(
					&self,
				) -> ::subxt::constants::StaticConstantAddress<
					::subxt::metadata::DecodeStaticType<::core::primitive::u8>,
				> {
					::subxt::constants::StaticConstantAddress::new(
						"TransactionPayment",
						"OperationalFeeMultiplier",
						[
							141u8, 130u8, 11u8, 35u8, 226u8, 114u8, 92u8, 179u8, 168u8, 110u8,
							28u8, 91u8, 221u8, 64u8, 4u8, 148u8, 201u8, 193u8, 185u8, 66u8, 226u8,
							114u8, 97u8, 79u8, 62u8, 212u8, 202u8, 114u8, 237u8, 228u8, 183u8,
							165u8,
						],
					)
				}
			}
		}

$ cargo test -- --ignored
[SNIP]
running 1 test
test src/lib.rs - node_runtime::transaction_payment::constants::ConstantsApi::operational_fee_multiplier (line 22) ... FAILED

failures:

---- src/lib.rs - node_runtime::transaction_payment::constants::ConstantsApi::operational_fee_multiplier (line 22) stdout ----
error[E0425]: cannot find value `tip` in this scope
 --> src/lib.rs:24:30
  |
4 | let priority = priority_calc(tip);
  |                              ^^^ not found in this scope

error[E0425]: cannot find value `inclusion_fee` in this scope
 --> src/lib.rs:27:20
  |
7 | let virtual_tip = (inclusion_fee + tip) * OperationalFeeMultiplier;
  |                    ^^^^^^^^^^^^^ not found in this scope

error[E0425]: cannot find value `tip` in this scope
 --> src/lib.rs:27:36
  |
7 | let virtual_tip = (inclusion_fee + tip) * OperationalFeeMultiplier;
  |                                    ^^^ not found in this scope

error[E0425]: cannot find value `OperationalFeeMultiplier` in this scope
 --> src/lib.rs:27:43
  |
7 | let virtual_tip = (inclusion_fee + tip) * OperationalFeeMultiplier;
  |                                           ^^^^^^^^^^^^^^^^^^^^^^^^ not found in this scope

error[E0425]: cannot find value `tip` in this scope
 --> src/lib.rs:28:30
  |
8 | let priority = priority_calc(tip + virtual_tip);
  |                              ^^^ not found in this scope

error[E0425]: cannot find function `priority_calc` in this scope
 --> src/lib.rs:24:16
  |
4 | let priority = priority_calc(tip);
  |                ^^^^^^^^^^^^^ not found in this scope

error[E0425]: cannot find function `priority_calc` in this scope
 --> src/lib.rs:28:16
  |
8 | let priority = priority_calc(tip + virtual_tip);
  |                ^^^^^^^^^^^^^ not found in this scope

error: aborting due to 7 previous errors

If this is supposed to be pseudocode, then it should not be marked as rust (but as "```pseudo code" or something instead (which will make rustdoc ignore it)), and if this is supposed to be rust code, then it should include enough code to pass compilation.

[jsdw]

These docs come straight from the node/pallet code on which the TypeInfo macro is used, so it is a perfectly valid doc test in its original location, but of course when generated via codegen from the metadata it no longer makes sense.

I wouldn't want to have to try and parse/alter the generated docs for such things as that is likely to be brittle/arduous, so I wonder if there is a global annotation to ignore doctests in a module; they basically shouldn't be ran at all, ever, from generated code.

[jsdw]

This thread has some thoughts:

https://users.rust-lang.org/t/disabling-rustdoc-tests-for-module/50110/5

Looking at this I realise that the failing doc test is annotated with ignore already, which is supposed to be used to ignore running the test, so it feels like it is correctly annotated (syntactically valid but not runnable Rust code) and I guess I'm not shocked that it fails when you run with --ignored. But nevertheless, if we can add an attribtue to prevent running them more completely I am in favour of doing so.

[deepink-mas]

AFAIUI "rust,ignore" is for valid doctests that are too time consuming to always run. To really disable these tests they could be annotated with anything other than "rust", for example "broken" (or "ignore"?) would do the trick, but not "rust,ignore". But if you look at it another way, then transcluding doc comments at all is perhaps not that useful.

[jsdw]

Alas, we can't control the docs anyway, so ultimately I think we need to focus on managing how they are treated on our side. Doc comments aren't useful if you use the macro directly (at least in my IDE via rust analyser) but are perhaps useful in IDEs if you codegen (we actually added them in response to an older issue here: #503).

Potentially we could provide an attribtue to allow you to decide whether to generate docs or not. Otherwise I wonder what #[cfg(rustdoc)] on the generated code would do (but compile error if module used elsewhere, perhaps in other doc examples?). Offhand, rewriting the docs to change code samples might be the only other avenue, and I'm less keen on that one off hand.

[lexnv]

Potentially we could provide an attribtue to allow you to decide whether to generate docs or not

Yep, I like that idea better than parsing the docs and replacing links and all their variations.

We could temporarily disable the broken_intra_doc_links, and the codegen would do something like

#[allow(rustdoc::broken_intra_doc_links)]
// mod expanded by subxt macro
pub mod polkadot {...
}
#[warn(rustdoc::broken_intra_doc_links)]

Tho that would be a bit opinionated as it may override whatever the user decided for that file.

[jsdw]

Potentially we could provide an attribtue to allow you to decide whether to generate docs or not

Yep, I like that idea better than parsing the docs and replacing links and all their variations.

We could temporarily disable the broken_intra_doc_links, and the codegen would do something like

#[allow(rustdoc::broken_intra_doc_links)]
// mod expanded by subxt macro
pub mod polkadot {...
}
#[warn(rustdoc::broken_intra_doc_links)]

Tho that would be a bit opinionated as it may override whatever the user decided for that file.

I'm not sure how #[allow(rustdoc::broken_intra_doc_links)] would help, and anyway, the user can also add whatever other attribtues they want on the module and nowadays they should be preserved :)

Offhand I didn't find any useful attrs myself so I'd guess that allowing people to opt-in (or opt-out but I'm guessing opt-in is a good default) to docs is a decent way to go!

[jsdw]

Per #843, we'll no longer generate docs in the subxt macro by default, and for codegen you'll be able to opt out of docs.