From 7043d51df04a791777e9494ca6327518fd79e99f Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Tue, 18 Nov 2025 16:36:59 +0100 Subject: [PATCH] [ty] Add hyperlinks to rule codes in CLI (#21502) --- .../src/renderer/display_list.rs | 75 ++++++++++++++++--- .../src/renderer/mod.rs | 1 + .../src/renderer/stylesheet.rs | 2 + crates/ruff_annotate_snippets/src/snippet.rs | 15 +++- crates/ruff_db/src/diagnostic/mod.rs | 10 +++ crates/ruff_db/src/diagnostic/render.rs | 15 ++-- .../ruff_db/src/diagnostic/render/concise.rs | 22 +++++- crates/ruff_db/src/diagnostic/stylesheet.rs | 40 ++++++++++ crates/ty_python_semantic/src/diagnostic.rs | 8 +- crates/ty_python_semantic/src/lint.rs | 8 ++ crates/ty_python_semantic/src/suppression.rs | 1 + .../ty_python_semantic/src/types/context.rs | 31 ++++---- .../ty_server/src/server/api/diagnostics.rs | 14 ++-- 13 files changed, 195 insertions(+), 47 deletions(-) diff --git a/crates/ruff_annotate_snippets/src/renderer/display_list.rs b/crates/ruff_annotate_snippets/src/renderer/display_list.rs index fb88a26223..7507f88a22 100644 --- a/crates/ruff_annotate_snippets/src/renderer/display_list.rs +++ b/crates/ruff_annotate_snippets/src/renderer/display_list.rs @@ -31,7 +31,7 @@ //! styling. //! //! The above snippet has been built out of the following structure: -use crate::snippet; +use crate::{Id, snippet}; use std::cmp::{Reverse, max, min}; use std::collections::HashMap; use std::fmt::Display; @@ -189,6 +189,7 @@ impl DisplaySet<'_> { } Ok(()) } + fn format_annotation( &self, line_offset: usize, @@ -199,11 +200,13 @@ impl DisplaySet<'_> { ) -> fmt::Result { let hide_severity = annotation.annotation_type.is_none(); let color = get_annotation_style(&annotation.annotation_type, stylesheet); + let formatted_len = if let Some(id) = &annotation.id { + let id_len = id.id.len(); if hide_severity { - id.len() + id_len } else { - 2 + id.len() + annotation_type_len(&annotation.annotation_type) + 2 + id_len + annotation_type_len(&annotation.annotation_type) } } else { annotation_type_len(&annotation.annotation_type) @@ -256,9 +259,20 @@ impl DisplaySet<'_> { let annotation_type = annotation_type_str(&annotation.annotation_type); if let Some(id) = annotation.id { if hide_severity { - buffer.append(line_offset, &format!("{id} "), *stylesheet.error()); + buffer.append( + line_offset, + &format!("{id} ", id = fmt_with_hyperlink(id.id, id.url, stylesheet)), + *stylesheet.error(), + ); } else { - buffer.append(line_offset, &format!("{annotation_type}[{id}]"), *color); + buffer.append( + line_offset, + &format!( + "{annotation_type}[{id}]", + id = fmt_with_hyperlink(id.id, id.url, stylesheet) + ), + *color, + ); } } else { buffer.append(line_offset, annotation_type, *color); @@ -707,7 +721,7 @@ impl DisplaySet<'_> { let style = get_annotation_style(&annotation.annotation_type, stylesheet); let mut formatted_len = if let Some(id) = &annotation.annotation.id { - 2 + id.len() + 2 + id.id.len() + annotation_type_len(&annotation.annotation.annotation_type) } else { annotation_type_len(&annotation.annotation.annotation_type) @@ -724,7 +738,10 @@ impl DisplaySet<'_> { } else if formatted_len != 0 { formatted_len += 2; let id = match &annotation.annotation.id { - Some(id) => format!("[{id}]"), + Some(id) => format!( + "[{id}]", + id = fmt_with_hyperlink(&id.id, id.url, stylesheet) + ), None => String::new(), }; buffer.puts( @@ -827,7 +844,7 @@ impl DisplaySet<'_> { #[derive(Clone, Debug, PartialEq)] pub(crate) struct Annotation<'a> { pub(crate) annotation_type: DisplayAnnotationType, - pub(crate) id: Option<&'a str>, + pub(crate) id: Option>, pub(crate) label: Vec>, pub(crate) is_fixable: bool, } @@ -1140,7 +1157,7 @@ fn format_message<'m>( fn format_title<'a>( level: crate::Level, - id: Option<&'a str>, + id: Option>, label: &'a str, is_fixable: bool, ) -> DisplayLine<'a> { @@ -1158,7 +1175,7 @@ fn format_title<'a>( fn format_footer<'a>( level: crate::Level, - id: Option<&'a str>, + id: Option>, label: &'a str, ) -> Vec> { let mut result = vec![]; @@ -1706,6 +1723,7 @@ fn format_body<'m>( annotation: Annotation { annotation_type, id: None, + label: format_label(annotation.label, None), is_fixable: false, }, @@ -1887,3 +1905,40 @@ fn char_width(c: char) -> Option { unicode_width::UnicodeWidthChar::width(c) } } + +pub(super) fn fmt_with_hyperlink<'a, T>( + content: T, + url: Option<&'a str>, + stylesheet: &Stylesheet, +) -> impl std::fmt::Display + 'a +where + T: std::fmt::Display + 'a, +{ + struct FmtHyperlink<'a, T> { + content: T, + url: Option<&'a str>, + } + + impl std::fmt::Display for FmtHyperlink<'_, T> + where + T: std::fmt::Display, + { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if let Some(url) = self.url { + write!(f, "\x1B]8;;{url}\x1B\\")?; + } + + self.content.fmt(f)?; + + if self.url.is_some() { + f.write_str("\x1B]8;;\x1B\\")?; + } + + Ok(()) + } + } + + let url = if stylesheet.hyperlink { url } else { None }; + + FmtHyperlink { content, url } +} diff --git a/crates/ruff_annotate_snippets/src/renderer/mod.rs b/crates/ruff_annotate_snippets/src/renderer/mod.rs index 0ea77b98c3..7d3050bf0b 100644 --- a/crates/ruff_annotate_snippets/src/renderer/mod.rs +++ b/crates/ruff_annotate_snippets/src/renderer/mod.rs @@ -76,6 +76,7 @@ impl Renderer { } .effects(Effects::BOLD), none: Style::new(), + hyperlink: true, }, ..Self::plain() } diff --git a/crates/ruff_annotate_snippets/src/renderer/stylesheet.rs b/crates/ruff_annotate_snippets/src/renderer/stylesheet.rs index ee1ab93764..d9ec70d6d0 100644 --- a/crates/ruff_annotate_snippets/src/renderer/stylesheet.rs +++ b/crates/ruff_annotate_snippets/src/renderer/stylesheet.rs @@ -10,6 +10,7 @@ pub(crate) struct Stylesheet { pub(crate) line_no: Style, pub(crate) emphasis: Style, pub(crate) none: Style, + pub(crate) hyperlink: bool, } impl Default for Stylesheet { @@ -29,6 +30,7 @@ impl Stylesheet { line_no: Style::new(), emphasis: Style::new(), none: Style::new(), + hyperlink: false, } } } diff --git a/crates/ruff_annotate_snippets/src/snippet.rs b/crates/ruff_annotate_snippets/src/snippet.rs index 76d615189d..363f8b2558 100644 --- a/crates/ruff_annotate_snippets/src/snippet.rs +++ b/crates/ruff_annotate_snippets/src/snippet.rs @@ -12,13 +12,19 @@ use std::ops::Range; +#[derive(Copy, Clone, Debug, Default, PartialEq)] +pub(crate) struct Id<'a> { + pub(crate) id: &'a str, + pub(crate) url: Option<&'a str>, +} + /// Primary structure provided for formatting /// /// See [`Level::title`] to create a [`Message`] #[derive(Debug)] pub struct Message<'a> { pub(crate) level: Level, - pub(crate) id: Option<&'a str>, + pub(crate) id: Option>, pub(crate) title: &'a str, pub(crate) snippets: Vec>, pub(crate) footer: Vec>, @@ -28,7 +34,12 @@ pub struct Message<'a> { impl<'a> Message<'a> { pub fn id(mut self, id: &'a str) -> Self { - self.id = Some(id); + self.id = Some(Id { id, url: None }); + self + } + + pub fn id_with_url(mut self, id: &'a str, url: Option<&'a str>) -> Self { + self.id = Some(Id { id, url }); self } diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 0761c96acb..b9ba21e5c4 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -65,6 +65,7 @@ impl Diagnostic { severity, message: message.into_diagnostic_message(), custom_concise_message: None, + documentation_url: None, annotations: vec![], subs: vec![], fix: None, @@ -370,6 +371,14 @@ impl Diagnostic { .is_some_and(|fix| fix.applies(config.fix_applicability)) } + pub fn documentation_url(&self) -> Option<&str> { + self.inner.documentation_url.as_deref() + } + + pub fn set_documentation_url(&mut self, url: Option) { + Arc::make_mut(&mut self.inner).documentation_url = url; + } + /// Returns the offset of the parent statement for this diagnostic if it exists. /// /// This is primarily used for checking noqa/secondary code suppressions. @@ -544,6 +553,7 @@ impl Diagnostic { #[derive(Debug, Clone, Eq, PartialEq, Hash, get_size2::GetSize)] struct DiagnosticInner { id: DiagnosticId, + documentation_url: Option, severity: Severity, message: DiagnosticMessage, custom_concise_message: Option, diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index f7c618ae41..dd55fcc2d7 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -205,6 +205,7 @@ impl<'a> Resolved<'a> { struct ResolvedDiagnostic<'a> { level: AnnotateLevel, id: Option, + documentation_url: Option, message: String, annotations: Vec>, is_fixable: bool, @@ -240,12 +241,12 @@ impl<'a> ResolvedDiagnostic<'a> { // `DisplaySet::format_annotation` for both cases, but this is a small hack to improve // the formatting of syntax errors for now. This should also be kept consistent with the // concise formatting. - Some(diag.secondary_code().map_or_else( + diag.secondary_code().map_or_else( || format!("{id}:", id = diag.inner.id), |code| code.to_string(), - )) + ) } else { - Some(diag.inner.id.to_string()) + diag.inner.id.to_string() }; let level = if config.hide_severity { @@ -256,7 +257,8 @@ impl<'a> ResolvedDiagnostic<'a> { ResolvedDiagnostic { level, - id, + id: Some(id), + documentation_url: diag.documentation_url().map(ToString::to_string), message: diag.inner.message.as_str().to_string(), annotations, is_fixable: config.show_fix_status && diag.has_applicable_fix(config), @@ -287,6 +289,7 @@ impl<'a> ResolvedDiagnostic<'a> { ResolvedDiagnostic { level: diag.inner.severity.to_annotate(), id: None, + documentation_url: None, message: diag.inner.message.as_str().to_string(), annotations, is_fixable: false, @@ -385,6 +388,7 @@ impl<'a> ResolvedDiagnostic<'a> { RenderableDiagnostic { level: self.level, id: self.id.as_deref(), + documentation_url: self.documentation_url.as_deref(), message: &self.message, snippets_by_input, is_fixable: self.is_fixable, @@ -485,6 +489,7 @@ struct RenderableDiagnostic<'r> { /// An ID is always present for top-level diagnostics and always absent for /// sub-diagnostics. id: Option<&'r str>, + documentation_url: Option<&'r str>, /// The message emitted with the diagnostic, before any snippets are /// rendered. message: &'r str, @@ -519,7 +524,7 @@ impl RenderableDiagnostic<'_> { .is_fixable(self.is_fixable) .lineno_offset(self.header_offset); if let Some(id) = self.id { - message = message.id(id); + message = message.id_with_url(id, self.documentation_url); } message.snippets(snippets) } diff --git a/crates/ruff_db/src/diagnostic/render/concise.rs b/crates/ruff_db/src/diagnostic/render/concise.rs index 4bc8a9735e..0d8c477f65 100644 --- a/crates/ruff_db/src/diagnostic/render/concise.rs +++ b/crates/ruff_db/src/diagnostic/render/concise.rs @@ -1,6 +1,6 @@ use crate::diagnostic::{ Diagnostic, DisplayDiagnosticConfig, Severity, - stylesheet::{DiagnosticStylesheet, fmt_styled}, + stylesheet::{DiagnosticStylesheet, fmt_styled, fmt_with_hyperlink}, }; use super::FileResolver; @@ -62,18 +62,29 @@ impl<'a> ConciseRenderer<'a> { } write!(f, "{sep} ")?; } + if self.config.hide_severity { if let Some(code) = diag.secondary_code() { write!( f, "{code} ", - code = fmt_styled(code, stylesheet.secondary_code) + code = fmt_styled( + fmt_with_hyperlink(&code, diag.documentation_url(), &stylesheet), + stylesheet.secondary_code + ) )?; } else { write!( f, "{id}: ", - id = fmt_styled(diag.inner.id.as_str(), stylesheet.secondary_code) + id = fmt_styled( + fmt_with_hyperlink( + &diag.inner.id, + diag.documentation_url(), + &stylesheet + ), + stylesheet.secondary_code + ) )?; } if self.config.show_fix_status { @@ -93,7 +104,10 @@ impl<'a> ConciseRenderer<'a> { f, "{severity}[{id}] ", severity = fmt_styled(severity, severity_style), - id = fmt_styled(diag.id(), stylesheet.emphasis) + id = fmt_styled( + fmt_with_hyperlink(&diag.id(), diag.documentation_url(), &stylesheet), + stylesheet.emphasis + ) )?; } diff --git a/crates/ruff_db/src/diagnostic/stylesheet.rs b/crates/ruff_db/src/diagnostic/stylesheet.rs index 488e4352c9..ba7391549e 100644 --- a/crates/ruff_db/src/diagnostic/stylesheet.rs +++ b/crates/ruff_db/src/diagnostic/stylesheet.rs @@ -31,6 +31,43 @@ where FmtStyled { content, style } } +pub(super) fn fmt_with_hyperlink<'a, T>( + content: T, + url: Option<&'a str>, + stylesheet: &DiagnosticStylesheet, +) -> impl std::fmt::Display + 'a +where + T: std::fmt::Display + 'a, +{ + struct FmtHyperlink<'a, T> { + content: T, + url: Option<&'a str>, + } + + impl std::fmt::Display for FmtHyperlink<'_, T> + where + T: std::fmt::Display, + { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + if let Some(url) = self.url { + write!(f, "\x1B]8;;{url}\x1B\\")?; + } + + self.content.fmt(f)?; + + if self.url.is_some() { + f.write_str("\x1B]8;;\x1B\\")?; + } + + Ok(()) + } + } + + let url = if stylesheet.hyperlink { url } else { None }; + + FmtHyperlink { content, url } +} + #[derive(Clone, Debug)] pub struct DiagnosticStylesheet { pub(crate) error: Style, @@ -47,6 +84,7 @@ pub struct DiagnosticStylesheet { pub(crate) deletion: Style, pub(crate) insertion_line_no: Style, pub(crate) deletion_line_no: Style, + pub(crate) hyperlink: bool, } impl Default for DiagnosticStylesheet { @@ -74,6 +112,7 @@ impl DiagnosticStylesheet { deletion: AnsiColor::Red.on_default(), insertion_line_no: AnsiColor::Green.on_default().effects(Effects::BOLD), deletion_line_no: AnsiColor::Red.on_default().effects(Effects::BOLD), + hyperlink: true, } } @@ -93,6 +132,7 @@ impl DiagnosticStylesheet { deletion: Style::new(), insertion_line_no: Style::new(), deletion_line_no: Style::new(), + hyperlink: false, } } } diff --git a/crates/ty_python_semantic/src/diagnostic.rs b/crates/ty_python_semantic/src/diagnostic.rs index f58c90191c..a54e7b9083 100644 --- a/crates/ty_python_semantic/src/diagnostic.rs +++ b/crates/ty_python_semantic/src/diagnostic.rs @@ -1,3 +1,7 @@ +use crate::{Db, Program, PythonVersionWithSource}; +use ruff_db::diagnostic::{Annotation, Diagnostic, SubDiagnostic, SubDiagnosticSeverity}; +use std::fmt::Write; + /// Suggest a name from `existing_names` that is similar to `wrong_name`. pub(crate) fn did_you_mean, T: AsRef>( existing_names: impl Iterator, @@ -24,10 +28,6 @@ pub(crate) fn did_you_mean, T: AsRef>( .map(|(id, _)| id) } -use crate::{Db, Program, PythonVersionWithSource}; -use ruff_db::diagnostic::{Annotation, Diagnostic, SubDiagnostic, SubDiagnosticSeverity}; -use std::fmt::Write; - /// Add a subdiagnostic to `diagnostic` that explains why a certain Python version was inferred. /// /// ty can infer the Python version from various sources, such as command-line arguments, diff --git a/crates/ty_python_semantic/src/lint.rs b/crates/ty_python_semantic/src/lint.rs index 0eccfac326..f1774cbb40 100644 --- a/crates/ty_python_semantic/src/lint.rs +++ b/crates/ty_python_semantic/src/lint.rs @@ -116,6 +116,10 @@ impl LintMetadata { self.documentation_lines().join("\n") } + pub fn documentation_url(&self) -> String { + lint_documentation_url(self.name()) + } + pub fn default_level(&self) -> Level { self.default_level } @@ -133,6 +137,10 @@ impl LintMetadata { } } +pub fn lint_documentation_url(lint_name: LintName) -> String { + format!("https://ty.dev/rules#{lint_name}") +} + #[doc(hidden)] pub const fn lint_metadata_defaults() -> LintMetadata { LintMetadata { diff --git a/crates/ty_python_semantic/src/suppression.rs b/crates/ty_python_semantic/src/suppression.rs index 6c9edbec68..6cf1f407e1 100644 --- a/crates/ty_python_semantic/src/suppression.rs +++ b/crates/ty_python_semantic/src/suppression.rs @@ -298,6 +298,7 @@ impl<'a> CheckSuppressionsContext<'a> { let id = DiagnosticId::Lint(lint.name()); let mut diag = Diagnostic::new(id, severity, ""); + diag.set_documentation_url(Some(lint.documentation_url())); let span = Span::from(self.file).with_range(range); diag.annotate(Annotation::primary(span).message(message)); self.diagnostics.push(diag); diff --git a/crates/ty_python_semantic/src/types/context.rs b/crates/ty_python_semantic/src/types/context.rs index 95e5ce8741..6ef5afcdf5 100644 --- a/crates/ty_python_semantic/src/types/context.rs +++ b/crates/ty_python_semantic/src/types/context.rs @@ -11,7 +11,7 @@ use ruff_text_size::{Ranged, TextRange}; use super::{Type, TypeCheckDiagnostics, binding_type}; -use crate::lint::LintSource; +use crate::lint::{LintSource, lint_documentation_url}; use crate::semantic_index::scope::ScopeId; use crate::semantic_index::semantic_index; use crate::types::function::FunctionDecorators; @@ -103,7 +103,7 @@ impl<'db, 'ast> InferContext<'db, 'ast> { } pub(super) fn is_lint_enabled(&self, lint: &'static LintMetadata) -> bool { - LintDiagnosticGuardBuilder::severity_and_source(self, lint).is_some() + LintDiagnosticGuardBuilder::severity_and_source(self, LintId::of(lint)).is_some() } /// Optionally return a builder for a lint diagnostic guard. @@ -395,7 +395,7 @@ impl Drop for LintDiagnosticGuard<'_, '_> { /// when the diagnostic is disabled or suppressed (among other reasons). pub(super) struct LintDiagnosticGuardBuilder<'db, 'ctx> { ctx: &'ctx InferContext<'db, 'ctx>, - id: DiagnosticId, + id: LintId, severity: Severity, source: LintSource, primary_span: Span, @@ -404,7 +404,7 @@ pub(super) struct LintDiagnosticGuardBuilder<'db, 'ctx> { impl<'db, 'ctx> LintDiagnosticGuardBuilder<'db, 'ctx> { fn severity_and_source( ctx: &'ctx InferContext<'db, 'ctx>, - lint: &'static LintMetadata, + lint: LintId, ) -> Option<(Severity, LintSource)> { // The comment below was copied from the original // implementation of diagnostic reporting. The code @@ -420,10 +420,9 @@ impl<'db, 'ctx> LintDiagnosticGuardBuilder<'db, 'ctx> { if !ctx.db.should_check_file(ctx.file) { return None; } - let lint_id = LintId::of(lint); // Skip over diagnostics if the rule // is disabled. - let (severity, source) = ctx.db.rule_selection(ctx.file).get(lint_id)?; + let (severity, source) = ctx.db.rule_selection(ctx.file).get(lint)?; // If we're not in type checking mode, // we can bail now. if ctx.is_in_no_type_check() { @@ -443,20 +442,20 @@ impl<'db, 'ctx> LintDiagnosticGuardBuilder<'db, 'ctx> { lint: &'static LintMetadata, range: TextRange, ) -> Option> { - let (severity, source) = Self::severity_and_source(ctx, lint)?; + let lint_id = LintId::of(lint); + + let (severity, source) = Self::severity_and_source(ctx, lint_id)?; let suppressions = suppressions(ctx.db(), ctx.file()); - let lint_id = LintId::of(lint); if let Some(suppression) = suppressions.find_suppression(range, lint_id) { ctx.diagnostics.borrow_mut().mark_used(suppression.id()); return None; } - let id = DiagnosticId::Lint(lint.name()); let primary_span = Span::from(ctx.file()).with_range(range); Some(LintDiagnosticGuardBuilder { ctx, - id, + id: lint_id, severity, source, primary_span, @@ -477,7 +476,8 @@ impl<'db, 'ctx> LintDiagnosticGuardBuilder<'db, 'ctx> { self, message: impl std::fmt::Display, ) -> LintDiagnosticGuard<'db, 'ctx> { - let mut diag = Diagnostic::new(self.id, self.severity, message); + let mut diag = Diagnostic::new(DiagnosticId::Lint(self.id.name()), self.severity, message); + diag.set_documentation_url(Some(self.id.documentation_url())); // This is why `LintDiagnosticGuard::set_primary_message` exists. // We add the primary annotation here (because it's required), but // the optional message can be added later. We could accept it here @@ -629,10 +629,15 @@ impl<'db, 'ctx> DiagnosticGuardBuilder<'db, 'ctx> { self, message: impl std::fmt::Display, ) -> DiagnosticGuard<'db, 'ctx> { - let diag = Some(Diagnostic::new(self.id, self.severity, message)); + let mut diag = Diagnostic::new(self.id, self.severity, message); + + if let DiagnosticId::Lint(lint_name) = diag.id() { + diag.set_documentation_url(Some(lint_documentation_url(lint_name))); + } + DiagnosticGuard { ctx: self.ctx, - diag, + diag: Some(diag), } } } diff --git a/crates/ty_server/src/server/api/diagnostics.rs b/crates/ty_server/src/server/api/diagnostics.rs index 8d6f62bdf3..9d228ec386 100644 --- a/crates/ty_server/src/server/api/diagnostics.rs +++ b/crates/ty_server/src/server/api/diagnostics.rs @@ -320,15 +320,11 @@ pub(super) fn to_lsp_diagnostic( }) .filter(|mapped_tags| !mapped_tags.is_empty()); - let code_description = diagnostic - .id() - .is_lint() - .then(|| { - Some(CodeDescription { - href: Url::parse(&format!("https://ty.dev/rules#{}", diagnostic.id())).ok()?, - }) - }) - .flatten(); + let code_description = diagnostic.documentation_url().and_then(|url| { + let href = Url::parse(url).ok()?; + + Some(CodeDescription { href }) + }); let mut related_information = Vec::new();