Skip to content

fix: config-docs-generator uninlined-format-args and collapsible-if clippy warnings #6359

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.

Sign up for GitHub

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

Merged
aldur merged 5 commits into from
Aug 11, 2025
Merged

fix: config-docs-generator uninlined-format-args and collapsible-if clippy warnings #6359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6f52cf6
fix uninlined format args
Jiloc Aug 7, 2025
2e8db53
Merge branch 'develop' into fix/config-docs-generator-uninlined-forma…
Jiloc Aug 7, 2025
97daaed
remove workaround for mismatched-lifetime-syntaxes
Jiloc Aug 7, 2025
2803d0a
cargo fmt
Jiloc Aug 7, 2025
2578c7d
fix collapsible_if clippy warning
Jiloc Aug 7, 2025
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
6 changes: 0 additions & 6 deletions contrib/tools/config-docs-generator/generate-config-docs.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,6 @@ main() {

cd "$PROJECT_ROOT"

# Workaround for new nightly lint that breaks stacks-common build.
# Allow callers to override or extend, but default to allowing the lint so documentation generation
# stays green until codebase is updated.
# TODO: Remove this once codebase will be updated to use the new lifetime syntax.
export RUSTFLAGS="${RUSTFLAGS:-} -A mismatched-lifetime-syntaxes"

# Step 1: Build the documentation generation tools
if [[ "$SKIP_BUILD" != "true" ]]; then
log_info "Building documentation generation tools..."
Expand Down
129 changes: 58 additions & 71 deletions contrib/tools/config-docs-generator/src/extract_docs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ fn main() -> Result<()> {
// Write the extracted docs to file
fs::write(output_file, serde_json::to_string_pretty(&config_docs)?)?;

println!("Successfully extracted documentation to {}", output_file);
println!("Successfully extracted documentation to {output_file}");
println!(
"Found {} structs with documentation",
config_docs.structs.len()
Expand Down Expand Up @@ -183,10 +183,7 @@ fn generate_rustdoc_json(package: &str) -> Result<serde_json::Value> {

// Generate rustdoc for additional crates that might contain referenced constants
for additional_crate in &additional_crates {
let error_msg = format!(
"Failed to run cargo rustdoc command for {}",
additional_crate
);
let error_msg = format!("Failed to run cargo rustdoc command for {additional_crate}");
let output = StdCommand::new("cargo")
.args([
"+nightly",
Expand All @@ -208,10 +205,7 @@ fn generate_rustdoc_json(package: &str) -> Result<serde_json::Value> {

if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
eprintln!(
"Warning: Failed to generate rustdoc for {}: {}",
additional_crate, stderr
);
eprintln!("Warning: Failed to generate rustdoc for {additional_crate}: {stderr}");
}
}

Expand All @@ -225,7 +219,7 @@ fn generate_rustdoc_json(package: &str) -> Result<serde_json::Value> {
};

// Read the generated JSON file - rustdoc generates it based on library name
let json_file_path = format!("{}/doc/{}.json", rustdoc_target_dir, lib_name);
let json_file_path = format!("{rustdoc_target_dir}/doc/{lib_name}.json");
let json_content = std::fs::read_to_string(json_file_path)
.context("Failed to read generated rustdoc JSON file")?;

Expand All @@ -249,10 +243,10 @@ fn extract_config_docs_from_rustdoc(
// Check if this item is a struct by looking for the "struct" field
if get_json_object(item, &["inner", "struct"]).is_some() {
// Check if this struct is in our target list (if specified)
if let Some(targets) = target_structs {
if !targets.contains(&name.to_string()) {
continue;
}
if let Some(targets) = target_structs
&& !targets.contains(&name.to_string())
{
continue;
}

let (struct_doc_opt, referenced_constants) =
Expand Down Expand Up @@ -464,8 +458,7 @@ fn parse_field_documentation(
"" => false, // Empty string defaults to false
text => text.parse::<bool>().unwrap_or_else(|_| {
eprintln!(
"Warning: Invalid @required value '{}' for field '{}', defaulting to false",
text, field_name
"Warning: Invalid @required value '{text}' for field '{field_name}', defaulting to false"
);
false
}),
Expand Down Expand Up @@ -632,14 +625,14 @@ fn parse_folded_block_scalar(lines: &[&str], _base_indent: usize) -> String {
// Remove trailing empty lines but preserve a single trailing newline if content exists
let trimmed = result.trim_end_matches('\n');
if !trimmed.is_empty() && result.ends_with('\n') {
format!("{}\n", trimmed)
format!("{trimmed}\n")
} else {
trimmed.to_string()
}
}

fn extract_annotation(metadata_section: &str, annotation_name: &str) -> Option<String> {
let annotation_pattern = format!("@{}:", annotation_name);
let annotation_pattern = format!("@{annotation_name}:");

if let Some(_start_pos) = metadata_section.find(&annotation_pattern) {
// Split the metadata section into lines for processing
Expand Down Expand Up @@ -820,15 +813,13 @@ fn resolve_constant_reference(
let additional_crate_libs = ["stacks_common"]; // Library names for additional crates

for lib_name in &additional_crate_libs {
let json_file_path = format!("target/rustdoc-json/doc/{}.json", lib_name);
if let Ok(json_content) = std::fs::read_to_string(&json_file_path) {
if let Ok(rustdoc_json) = serde_json::from_str::<serde_json::Value>(&json_content) {
if let Some(index) = get_json_object(&rustdoc_json, &["index"]) {
if let Some(value) = resolve_constant_in_index(name, index) {
return Some(value);
}
}
}
let json_file_path = format!("target/rustdoc-json/doc/{lib_name}.json");
if let Ok(json_content) = std::fs::read_to_string(&json_file_path)
&& let Ok(rustdoc_json) = serde_json::from_str::<serde_json::Value>(&json_content)
&& let Some(index) = get_json_object(&rustdoc_json, &["index"])
&& let Some(value) = resolve_constant_in_index(name, index)
{
return Some(value);
}
}

Expand All @@ -842,61 +833,57 @@ fn resolve_constant_in_index(
// Look for a constant with the given name in the rustdoc index
for (_item_id, item) in rustdoc_index {
// Check if this item's name matches the constant we're looking for
if let Some(item_name) = get_json_string(item, &["name"]) {
if item_name == name {
// Check if this item is a constant by looking for the "constant" field
if let Some(constant_data) = get_json_object(item, &["inner", "constant"]) {
// Try newer rustdoc JSON structure first (with nested 'const' field)
let constant_data_value = serde_json::Value::Object(constant_data.clone());
if get_json_object(&constant_data_value, &["const"]).is_some() {
// For literal constants, prefer expr which doesn't have type suffix
if get_json_path(&constant_data_value, &["const", "is_literal"])
.and_then(|v| v.as_bool())
== Some(true)
{
// Access the expression field for literal constant values
if let Some(expr) =
get_json_string(&constant_data_value, &["const", "expr"])
{
if expr != "_" {
return Some(expr.to_string());
}
}
}

// For computed constants or when expr is "_", use value but strip type suffix
if let Some(value) =
get_json_string(&constant_data_value, &["const", "value"])
{
return Some(strip_type_suffix(value));
}

// Fallback to expr if value is not available
if let Some(item_name) = get_json_string(item, &["name"])
&& item_name == name
{
// Check if this item is a constant by looking for the "constant" field
if let Some(constant_data) = get_json_object(item, &["inner", "constant"]) {
// Try newer rustdoc JSON structure first (with nested 'const' field)
let constant_data_value = serde_json::Value::Object(constant_data.clone());
if get_json_object(&constant_data_value, &["const"]).is_some() {
// For literal constants, prefer expr which doesn't have type suffix
if get_json_path(&constant_data_value, &["const", "is_literal"])
.and_then(|v| v.as_bool())
== Some(true)
{
// Access the expression field for literal constant values
if let Some(expr) =
get_json_string(&constant_data_value, &["const", "expr"])
&& expr != "_"
{
if expr != "_" {
return Some(expr.to_string());
}
return Some(expr.to_string());
}
}

// Fall back to older rustdoc JSON structure for compatibility
if let Some(value) = get_json_string(&constant_data_value, &["value"]) {
// For computed constants or when expr is "_", use value but strip type suffix
if let Some(value) = get_json_string(&constant_data_value, &["const", "value"])
{
return Some(strip_type_suffix(value));
}
if let Some(expr) = get_json_string(&constant_data_value, &["expr"]) {
if expr != "_" {
return Some(expr.to_string());
}
}

// For some constants, the value might be in the type field if it's a simple literal
if let Some(type_str) = get_json_string(&constant_data_value, &["type"]) {
// Handle simple numeric or string literals embedded in type
return Some(type_str.to_string());
// Fallback to expr if value is not available
if let Some(expr) = get_json_string(&constant_data_value, &["const", "expr"])
&& expr != "_"
{
return Some(expr.to_string());
}
}

// Fall back to older rustdoc JSON structure for compatibility
if let Some(value) = get_json_string(&constant_data_value, &["value"]) {
return Some(strip_type_suffix(value));
}
if let Some(expr) = get_json_string(&constant_data_value, &["expr"])
&& expr != "_"
{
return Some(expr.to_string());
}

// For some constants, the value might be in the type field if it's a simple literal
if let Some(type_str) = get_json_string(&constant_data_value, &["type"]) {
// Handle simple numeric or string literals embedded in type
return Some(type_str.to_string());
}
}
}
}
Expand Down
48 changes: 18 additions & 30 deletions contrib/tools/config-docs-generator/src/generate_markdown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ fn main() -> Result<()> {
let mappings_path = matches.get_one::<String>("mappings").unwrap();

let input_content = fs::read_to_string(input_path)
.with_context(|| format!("Failed to read input JSON file: {}", input_path))?;
.with_context(|| format!("Failed to read input JSON file: {input_path}"))?;

let config_docs: ConfigDocs =
serde_json::from_str(&input_content).with_context(|| "Failed to parse input JSON")?;
Expand All @@ -113,43 +113,32 @@ fn main() -> Result<()> {
let markdown = generate_markdown(&config_docs, template_path, &custom_mappings)?;

fs::write(output_path, markdown)
.with_context(|| format!("Failed to write output file: {}", output_path))?;
.with_context(|| format!("Failed to write output file: {output_path}"))?;

println!(
"Successfully generated Markdown documentation at {}",
output_path
);
println!("Successfully generated Markdown documentation at {output_path}");
Ok(())
}

fn load_section_name_mappings(mappings_file: &str) -> Result<HashMap<String, String>> {
let content = fs::read_to_string(mappings_file).with_context(|| {
format!(
"Failed to read section name mappings file: {}",
mappings_file
)
})?;
let content = fs::read_to_string(mappings_file)
.with_context(|| format!("Failed to read section name mappings file: {mappings_file}"))?;

let mappings: HashMap<String, String> = serde_json::from_str(&content).with_context(|| {
format!(
"Failed to parse section name mappings JSON: {}",
mappings_file
)
})?;
let mappings: HashMap<String, String> = serde_json::from_str(&content)
.with_context(|| format!("Failed to parse section name mappings JSON: {mappings_file}"))?;

Ok(mappings)
}

fn load_template(template_path: &str) -> Result<String> {
fs::read_to_string(template_path)
.with_context(|| format!("Failed to read template file: {}", template_path))
.with_context(|| format!("Failed to read template file: {template_path}"))
}

fn render_template(template: &str, variables: HashMap<String, String>) -> String {
let mut result = template.to_string();

for (key, value) in variables {
let placeholder = format!("{{{{{}}}}}", key);
let placeholder = format!("{{{{{key}}}}}");
result = result.replace(&placeholder, &value);
}

Expand Down Expand Up @@ -245,7 +234,7 @@ fn generate_struct_section(
custom_mappings: &HashMap<String, String>,
) -> Result<()> {
let section_name = struct_to_section_name(&struct_doc.name, custom_mappings);
output.push_str(&format!("## {}\n\n", section_name));
output.push_str(&format!("## {section_name}\n\n"));

// Add struct description if available
if let Some(description) = &struct_doc.description {
Expand Down Expand Up @@ -365,7 +354,7 @@ fn generate_field_row(

// Add deprecation warning if present
if let Some(deprecated) = &field.deprecated {
description_parts.push(format!("<br><br>**⚠️ DEPRECATED:** {}", deprecated));
description_parts.push(format!("<br><br>**⚠️ DEPRECATED:** {deprecated}"));
}

// Add TOML example if present
Expand All @@ -385,16 +374,15 @@ fn generate_field_row(
.replace('\n', "&#10;"); // Use HTML entity for newline to avoid <br> conversion

let example_section = format!(
"<br><br>**Example:**<br><pre><code>{}</code></pre>",
escaped_example // HTML entities will be rendered as newlines by <pre>
"<br><br>**Example:**<br><pre><code>{escaped_example}</code></pre>" // HTML entities will be rendered as newlines by <pre>
);
description_parts.push(example_section);
}

// Add units information if present
if let Some(units) = &field.units {
let units_text = process_intralinks_with_context(units, global_context, struct_name);
description_parts.push(format!("<br><br>**Units:** {}", units_text));
description_parts.push(format!("<br><br>**Units:** {units_text}"));
}

let description = if description_parts.is_empty() {
Expand Down Expand Up @@ -513,11 +501,11 @@ fn process_reference(
// Check if it's the same struct or different struct
if ref_struct_name == current_struct_name {
// Same struct: just show field name
return format!("[{}](#{}) ", field_name, anchor_id);
return format!("[{field_name}](#{anchor_id}) ");
} else {
// Different struct: show [config_section].field_name as a link
let config_section = section_name.trim_start_matches('[').trim_end_matches(']');
return format!("[[{}].{}](#{}) ", config_section, field_name, anchor_id);
return format!("[[{config_section}].{field_name}](#{anchor_id}) ");
}
}
}
Expand All @@ -540,11 +528,11 @@ fn process_reference(
// Check if it's the same struct or different struct
if field_struct_name == current_struct_name {
// Same struct: just show field name
return format!("[{}](#{}) ", reference, anchor_id);
return format!("[{reference}](#{anchor_id}) ");
} else {
// Different struct: show [config_section].field_name as a link
let config_section = section_name.trim_start_matches('[').trim_end_matches(']');
return format!("[[{}].{}](#{}) ", config_section, reference, anchor_id);
return format!("[[{config_section}].{reference}](#{anchor_id}) ");
}
}
}
Expand Down Expand Up @@ -577,7 +565,7 @@ fn process_hierarchical_lists(
let processed_content =
process_intralinks_with_context(content, global_context, struct_name);

result.push(format!("{}{}", indent_html, processed_content));
result.push(format!("{indent_html}{processed_content}"));
} else {
// Process intra-links in non-bullet lines too
let processed_line = process_intralinks_with_context(line, global_context, struct_name);
Expand Down