Avoid retaining rule documentation for diagnostic URLs (#29517)

crates/ruff/src/commands/rule.rs

@@ -22,7 +22,7 @@ struct Explanation<'a> {
     fix: String,
     fix_availability: FixAvailability,
     #[expect(clippy::struct_field_names)]
-    explanation: Option<&'a str>,
+    explanation: &'a str,
     preview: bool,
     status: RuleStatus,
     category: Category,
@@ -94,15 +94,7 @@ fn format_rule_text(rule: Rule) -> String {
         output.push('\n');
     }
 
-    if let Some(explanation) = rule.explanation() {
-        output.push_str(explanation.trim());
-    } else {
-        output.push_str("Message formats:");
-        for format in rule.message_formats() {
-            output.push('\n');
-            let _ = write!(&mut output, "* {format}");
-        }
-    }
+    output.push_str(rule.explanation().trim());
     output
 }
 

crates/ruff_dev/src/generate_docs.rs

@@ -27,137 +27,134 @@ pub(crate) struct Args {
 
 pub(crate) fn main(args: &Args) -> Result<()> {
     for rule in Rule::iter() {
-        if let Some(explanation) = rule.explanation() {
-            let mut output = String::new();
+        let explanation = rule.explanation();
+        let mut output = String::new();
 
-            let _ = writeln!(&mut output, "# {}", rule.name_and_code());
+        let _ = writeln!(&mut output, "# {}", rule.name_and_code());
 
-            let status_text = match rule.status() {
-                RuleStatus::Stable { since } => {
-                    format!(
-                        r#"Added in <a href="https://github.com/astral-sh/ruff/releases/tag/{since}">{since}</a>"#
-                    )
-                }
-                RuleStatus::Preview { since } => {
-                    format!(
-                        r#"Preview (since <a href="https://github.com/astral-sh/ruff/releases/tag/{since}">{since}</a>)"#
-                    )
-                }
-                RuleStatus::Deprecated { since } => {
-                    format!(
-                        r#"Deprecated (since <a href="https://github.com/astral-sh/ruff/releases/tag/{since}">{since}</a>)"#
-                    )
-                }
-                RuleStatus::Removed { since } => {
-                    format!(
-                        r#"Removed (since <a href="https://github.com/astral-sh/ruff/releases/tag/{since}">{since}</a>)"#
-                    )
-                }
-            };
+        let status_text = match rule.status() {
+            RuleStatus::Stable { since } => {
+                format!(
+                    r#"Added in <a href="https://github.com/astral-sh/ruff/releases/tag/{since}">{since}</a>"#
+                )
+            }
+            RuleStatus::Preview { since } => {
+                format!(
+                    r#"Preview (since <a href="https://github.com/astral-sh/ruff/releases/tag/{since}">{since}</a>)"#
+                )
+            }
+            RuleStatus::Deprecated { since } => {
+                format!(
+                    r#"Deprecated (since <a href="https://github.com/astral-sh/ruff/releases/tag/{since}">{since}</a>)"#
+                )
+            }
+            RuleStatus::Removed { since } => {
+                format!(
+                    r#"Removed (since <a href="https://github.com/astral-sh/ruff/releases/tag/{since}">{since}</a>)"#
+                )
+            }
+        };
 
-            let issue_search = format!(
-                "(%27{encoded_name}%27{code})",
-                encoded_name =
-                    url::form_urlencoded::byte_serialize(rule.name().as_str().as_bytes())
-                        .collect::<String>(),
-                code = rule
-                    .noqa_code()
-                    .map(|code| format!("%20OR%20{code}"))
-                    .unwrap_or_default(),
-            );
+        let issue_search = format!(
+            "(%27{encoded_name}%27{code})",
+            encoded_name = url::form_urlencoded::byte_serialize(rule.name().as_str().as_bytes())
+                .collect::<String>(),
+            code = rule
+                .noqa_code()
+                .map(|code| format!("%20OR%20{code}"))
+                .unwrap_or_default(),
+        );
 
-            let _ = writeln!(
-                &mut output,
-                r#"<small>
+        let _ = writeln!(
+            &mut output,
+            r#"<small>
 {status_text} ·
 <a href="https://github.com/astral-sh/ruff/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20{issue_search}" target="_blank">Related issues</a> ·
 <a href="https://github.com/astral-sh/ruff/blob/main/{file}#L{line}" target="_blank">View source</a>
 </small>
 
 "#,
-                file =
-                    url::form_urlencoded::byte_serialize(rule.file().replace('\\', "/").as_bytes())
-                        .collect::<String>(),
-                line = rule.line(),
+            file = url::form_urlencoded::byte_serialize(rule.file().replace('\\', "/").as_bytes())
+                .collect::<String>(),
+            line = rule.line(),
+        );
+        if let Some(linter) = rule
+            .noqa_code()
+            .and_then(|code| Linter::parse_code(&code.to_string()).map(|(linter, _)| linter))
+            .filter(|linter| linter.url().is_some())
+        {
+            let common_prefix: String = match linter.common_prefix() {
+                "" => linter
+                    .upstream_categories()
+                    .unwrap()
+                    .iter()
+                    .map(|c| c.prefix)
+                    .join("-"),
+                prefix => prefix.to_string(),
+            };
+            let anchor = format!(
+                "{}-{}",
+                linter.name().to_lowercase(),
+                common_prefix.to_lowercase()
             );
-            if let Some(linter) = rule
-                .noqa_code()
-                .and_then(|code| Linter::parse_code(&code.to_string()).map(|(linter, _)| linter))
-                .filter(|linter| linter.url().is_some())
-            {
-                let common_prefix: String = match linter.common_prefix() {
-                    "" => linter
-                        .upstream_categories()
-                        .unwrap()
-                        .iter()
-                        .map(|c| c.prefix)
-                        .join("-"),
-                    prefix => prefix.to_string(),
-                };
-                let anchor = format!(
-                    "{}-{}",
-                    linter.name().to_lowercase(),
-                    common_prefix.to_lowercase()
-                );
 
-                let _ = write!(
-                    output,
-                    "Derived from the **[{}](../rules.md#{})** linter.",
-                    linter.name(),
-                    anchor,
-                );
-                output.push('\n');
-                output.push('\n');
-            }
+            let _ = write!(
+                output,
+                "Derived from the **[{}](../rules.md#{})** linter.",
+                linter.name(),
+                anchor,
+            );
+            output.push('\n');
+            output.push('\n');
+        }
 
-            if rule.is_deprecated() {
-                output.push_str(
-                    r"**Warning: This rule is deprecated and will be removed in a future release.**",
-                );
-                output.push('\n');
-                output.push('\n');
-            }
+        if rule.is_deprecated() {
+            output.push_str(
+                r"**Warning: This rule is deprecated and will be removed in a future release.**",
+            );
+            output.push('\n');
+            output.push('\n');
+        }
 
-            if rule.is_removed() {
-                output.push_str(
-                    r"**Warning: This rule has been removed and its documentation is only available for historical reasons.**",
-                );
-                output.push('\n');
-                output.push('\n');
-            }
+        if rule.is_removed() {
+            output.push_str(
+                r"**Warning: This rule has been removed and its documentation is only available for historical reasons.**",
+            );
+            output.push('\n');
+            output.push('\n');
+        }
 
-            let fix_availability = rule.fixable();
-            if matches!(
-                fix_availability,
-                FixAvailability::Always | FixAvailability::Sometimes
-            ) {
-                output.push_str(&fix_availability.to_string());
-                output.push('\n');
-                output.push('\n');
-            }
+        let fix_availability = rule.fixable();
+        if matches!(
+            fix_availability,
+            FixAvailability::Always | FixAvailability::Sometimes
+        ) {
+            output.push_str(&fix_availability.to_string());
+            output.push('\n');
+            output.push('\n');
+        }
 
-            if rule.is_preview() {
-                output.push_str(
-                    r"This rule is unstable and in [preview](../preview.md). The `--preview` flag is required for use.",
-                );
-                output.push('\n');
-                output.push('\n');
-            }
+        if rule.is_preview() {
+            output.push_str(
+                r"This rule is unstable and in [preview](../preview.md). The `--preview` flag is required for use.",
+            );
+            output.push('\n');
+            output.push('\n');
+        }
 
-            process_documentation(explanation.trim(), &mut output, rule.name().as_str());
+        process_documentation(explanation.trim(), &mut output, rule.name().as_str());
 
-            let filename = PathBuf::from(ROOT_DIR)
-                .join("docs")
-                .join("rules")
-                .join(&*rule.name())
-                .with_extension("md");
+        let filename = PathBuf::from(ROOT_DIR)
+            .join("docs")
+            .join("rules")
+            .join(&*rule.name())
+            .with_extension("md");
 
-            if args.dry_run {
-                println!("{output}");
-            } else {
-                fs::create_dir_all("docs/rules")?;
-                fs::write(filename, output)?;
-            }
+        if args.dry_run {
+            println!("{output}");
+        } else {
+            fs::create_dir_all("docs/rules")?;
+            fs::write(filename, output)?;
         }
     }
     Ok(())

crates/ruff_dev/src/generate_rules_table.rs

@@ -170,19 +170,13 @@ fn generate_table(
 
         // Message placeholders can be mistaken for an attribute list on the table cell.
         // Keep the identity class on an HTML wrapper so messages retain their Markdown.
-        #[expect(clippy::or_fun_call)]
         let _ = write!(
             table_out,
-            "| <span class='rule-identity'>{ss}{explanation}<br>{message}{se}</span> \
+            "| <span class='rule-identity'>{ss}[`{rule_name}`](rules/{rule_name}.md)<br>{message}{se}</span> \
              | {ss}{category}{se} {{ .rule-category }} \
              | {ss}{linter_name}{se} {{ {linter_anchor} .rule-linter data-linter='{linter_slug}' }} \
              | <div {SYMBOLS_CONTAINER}>{status_token}{fix_token}{default_token}</div> {{ .rule-status data-status='{status}' }} |",
             category = rule.category(),
-            explanation = rule
-                .explanation()
-                .is_some()
-                .then_some(format_args!("[`{rule_name}`](rules/{rule_name}.md)"))
-                .unwrap_or(format_args!("`{rule_name}`")),
         );
         table_out.push('\n');
     }

crates/ruff_linter/src/message/mod.rs

@@ -128,7 +128,7 @@ where
     if let Some(code) = rule.noqa_code() {
         diagnostic.set_secondary_code(SecondaryCode::new(code.to_string()));
     }
-    diagnostic.set_documentation_url(rule.url());
+    diagnostic.set_documentation_url(Some(rule.url()));
 
     diagnostic
 }

crates/ruff_linter/src/message/sarif.rs

@@ -117,11 +117,9 @@ struct SarifDriver<'a> {
 #[derive(Debug, Clone, Serialize)]
 #[serde(rename_all = "camelCase")]
 struct SarifRule<'a> {
-    #[serde(skip_serializing_if = "Option::is_none")]
-    full_description: Option<SarifMessage<'a>>,
+    full_description: SarifMessage<'a>,
     help: SarifMessage<'a>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    help_uri: Option<String>,
+    help_uri: String,
     id: &'a str,
     properties: SarifProperties<'a>,
     short_description: SarifMessage<'a>,
@@ -150,9 +148,9 @@ impl<'a> From<(&'a str, SarifLevel)> for SarifRule<'a> {
             short_description: SarifMessage {
                 text: rule.message_formats()[0].into(),
             },
-            full_description: rule
-                .explanation()
-                .map(|text| SarifMessage { text: text.into() }),
+            full_description: SarifMessage {
+                text: rule.explanation().into(),
+            },
             help: SarifMessage {
                 text: rule.message_formats()[0].into(),
             },

crates/ruff_linter/src/registry.rs

@@ -351,15 +351,15 @@ impl Rule {
         }
     }
 
-    /// Return the URL for the rule documentation, if it exists.
-    pub fn url(&self) -> Option<String> {
-        self.explanation().is_some().then(|| {
-            format!(
-                "{}/rules/{name}",
-                env!("CARGO_PKG_HOMEPAGE"),
-                name = self.name()
-            )
-        })
+    /// Return the URL for the rule documentation.
+    pub fn url(&self) -> String {
+        // Every rule has documentation. Calling `explanation()` here would retain all
+        // documentation strings in binaries that only need the URLs.
+        format!(
+            "{}/rules/{name}",
+            env!("CARGO_PKG_HOMEPAGE"),
+            name = self.name()
+        )
     }
 
     pub fn name(&self) -> LintName {
@@ -485,7 +485,7 @@ mod tests {
     fn documentation() {
         for rule in Rule::iter() {
             assert!(
-                rule.explanation().is_some(),
+                !rule.explanation().trim().is_empty(),
                 "Rule {} is missing documentation",
                 rule.name()
             );

crates/ruff_linter/src/violation.rs

@@ -34,7 +34,7 @@ pub trait ViolationMetadata {
 
     /// Returns an explanation of what this violation catches,
     /// why it's bad, and what users should do instead.
-    fn explain() -> Option<&'static str>;
+    fn explain() -> &'static str;
 
     /// Returns the rule status for this violation.
     fn status() -> RuleStatus;

crates/ruff_macros/src/map_codes.rs

@@ -484,7 +484,7 @@ fn register_rules<'a>(input: impl Iterator<Item = &'a Rule>) -> TokenStream {
             }
 
             /// Returns the documentation for this rule.
-            pub fn explanation(&self) -> Option<&'static str> {
+            pub fn explanation(&self) -> &'static str {
                 use crate::ViolationMetadata;
                 match self { #rule_explanation_match_arms }
             }

crates/ruff_macros/src/violation_metadata.rs

@@ -36,8 +36,8 @@ pub(crate) fn violation_metadata(input: DeriveInput) -> syn::Result<TokenStream>
                 crate::registry::Rule::#name
             }
 
-            fn explain() -> Option<&'static str> {
-                Some(#docs)
+            fn explain() -> &'static str {
+                #docs
             }
 
             fn status() -> crate::codes::RuleStatus {

crates/ruff_server/src/server/api/requests/hover.rs

@@ -145,11 +145,6 @@ fn format_rule_text(rule: Rule) -> String {
         output.push('\n');
     }
 
-    if let Some(explanation) = rule.explanation() {
-        output.push_str(explanation.trim());
-    } else {
-        tracing::warn!("Rule {} does not have an explanation", rule.name_and_code());
-        output.push_str("An issue occurred: an explanation for this rule was not found.");
-    }
+    output.push_str(rule.explanation().trim());
     output
 }