Move fix suggestion to subdiagnostic (#19464)

Summary
--

This PR tweaks Ruff's internal usage of the new diagnostic model to more
closely
match the intended use, as I understand it. Specifically, it moves the
fix/help
suggestion from the primary annotation's message to a subdiagnostic. In
turn, it
adds the secondary/noqa code as the new primary annotation message. As
shown in
the new `ruff_db` tests, this more closely mirrors Ruff's current
diagnostic
output.

I also added `Severity::Help` to render the fix suggestion with a
`help:` prefix
instead of `info:`.

These changes don't have any external impact now but should help a bit
with #19415.

Test Plan
--

New full output format tests in `ruff_db`

Rendered Diagnostics
--

Full diagnostic output from `annotate-snippets` in this PR:

``` 
error[unused-import]: `os` imported but unused
  --> fib.py:1:8
   |
 1 | import os
   |        ^^
   |
 help: Remove unused import: `os`
```

Current Ruff output for the same code:

```
fib.py:1:8: F401 [*] `os` imported but unused
  |
1 | import os
  |        ^^ F401
  |
  = help: Remove unused import: `os`
```

Proposed final output after #19415:

``` 
F401 [*] `os` imported but unused
  --> fib.py:1:8
   |
 1 | import os
   |        ^^
   |
 help: Remove unused import: `os`
```

These are slightly updated from
https://github.com/astral-sh/ruff/pull/19464#issuecomment-3097377634
below to remove the extra noqa codes in the primary annotation messages
for the first and third cases.
This commit is contained in:
Brent Westbrook 2025-07-22 10:03:58 -04:00 committed by GitHub
parent c82fa94e0a
commit fd335eb8b7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
20 changed files with 235 additions and 106 deletions

View file

@ -11,7 +11,7 @@ use diagnostic::{
INVALID_CONTEXT_MANAGER, INVALID_SUPER_ARGUMENT, NOT_ITERABLE, POSSIBLY_UNBOUND_IMPLICIT_CALL,
UNAVAILABLE_IMPLICIT_SUPER_ARGUMENTS,
};
use ruff_db::diagnostic::{Annotation, Diagnostic, Severity, Span, SubDiagnostic};
use ruff_db::diagnostic::{Annotation, Diagnostic, Span, SubDiagnostic, SubDiagnosticSeverity};
use ruff_db::files::File;
use ruff_python_ast::name::Name;
use ruff_python_ast::{self as ast, AnyNodeRef};
@ -7077,7 +7077,7 @@ impl<'db> BoolError<'db> {
not_boolable_type.display(context.db())
));
let mut sub = SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
"`__bool__` methods must only have a `self` parameter",
);
if let Some((func_span, parameter_span)) = not_boolable_type
@ -7102,7 +7102,7 @@ impl<'db> BoolError<'db> {
not_boolable = not_boolable_type.display(context.db()),
));
let mut sub = SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
format_args!(
"`{return_type}` is not assignable to `bool`",
return_type = return_type.display(context.db()),
@ -7128,7 +7128,7 @@ impl<'db> BoolError<'db> {
not_boolable_type.display(context.db())
));
let sub = SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
format_args!(
"`__bool__` on `{}` must be callable",
not_boolable_type.display(context.db())

View file

@ -30,7 +30,7 @@ use crate::types::{
MethodWrapperKind, PropertyInstanceType, SpecialFormType, TypeMapping, UnionType,
WrapperDescriptorKind, enums, ide_support, todo_type,
};
use ruff_db::diagnostic::{Annotation, Diagnostic, Severity, SubDiagnostic};
use ruff_db::diagnostic::{Annotation, Diagnostic, SubDiagnostic, SubDiagnosticSeverity};
use ruff_python_ast as ast;
/// Binding information for a possible union of callables. At a call site, the arguments must be
@ -1668,8 +1668,10 @@ impl<'db> CallableBinding<'db> {
.first()
.and_then(|overload| overload.spans(context.db()))
{
let mut sub =
SubDiagnostic::new(Severity::Info, "First overload defined here");
let mut sub = SubDiagnostic::new(
SubDiagnosticSeverity::Info,
"First overload defined here",
);
sub.annotate(Annotation::primary(spans.signature));
diag.sub(sub);
}
@ -1696,7 +1698,7 @@ impl<'db> CallableBinding<'db> {
implementation.and_then(|function| function.spans(context.db()))
{
let mut sub = SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
"Overload implementation defined here",
);
sub.annotate(Annotation::primary(spans.signature));
@ -2570,8 +2572,10 @@ impl<'db> BindingError<'db> {
overload.parameter_span(context.db(), Some(parameter.index))
})
{
let mut sub =
SubDiagnostic::new(Severity::Info, "Matching overload defined here");
let mut sub = SubDiagnostic::new(
SubDiagnosticSeverity::Info,
"Matching overload defined here",
);
sub.annotate(Annotation::primary(name_span));
sub.annotate(
Annotation::secondary(parameter_span)
@ -2607,7 +2611,8 @@ impl<'db> BindingError<'db> {
} else if let Some((name_span, parameter_span)) =
callable_ty.parameter_span(context.db(), Some(parameter.index))
{
let mut sub = SubDiagnostic::new(Severity::Info, "Function defined here");
let mut sub =
SubDiagnostic::new(SubDiagnosticSeverity::Info, "Function defined here");
sub.annotate(Annotation::primary(name_span));
sub.annotate(
Annotation::secondary(parameter_span).message("Parameter declared here"),
@ -2733,7 +2738,10 @@ impl<'db> BindingError<'db> {
let module = parsed_module(context.db(), typevar_definition.file(context.db()))
.load(context.db());
let typevar_range = typevar_definition.full_range(context.db(), &module);
let mut sub = SubDiagnostic::new(Severity::Info, "Type variable defined here");
let mut sub = SubDiagnostic::new(
SubDiagnosticSeverity::Info,
"Type variable defined here",
);
sub.annotate(Annotation::primary(typevar_range.into()));
diag.sub(sub);
}
@ -2801,7 +2809,7 @@ impl UnionDiagnostic<'_, '_> {
/// diagnostic.
fn add_union_context(&self, db: &'_ dyn Db, diag: &mut Diagnostic) {
let sub = SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
format_args!(
"Union variant `{callable_ty}` is incompatible with this call site",
callable_ty = self.binding.callable_type.display(db),
@ -2810,7 +2818,7 @@ impl UnionDiagnostic<'_, '_> {
diag.sub(sub);
let sub = SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
format_args!(
"Attempted to call union type `{}`",
self.callable_type.display(db)

View file

@ -1,7 +1,7 @@
use std::fmt;
use drop_bomb::DebugDropBomb;
use ruff_db::diagnostic::{DiagnosticTag, SubDiagnostic};
use ruff_db::diagnostic::{DiagnosticTag, SubDiagnostic, SubDiagnosticSeverity};
use ruff_db::parsed::ParsedModuleRef;
use ruff_db::{
diagnostic::{Annotation, Diagnostic, DiagnosticId, IntoDiagnosticMessage, Severity, Span},
@ -330,7 +330,7 @@ impl Drop for LintDiagnosticGuard<'_, '_> {
let mut diag = self.diag.take().unwrap();
diag.sub(SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
match self.source {
LintSource::Default => format!("rule `{}` is enabled by default", diag.id()),
LintSource::Cli => format!("rule `{}` was selected on the command line", diag.id()),

View file

@ -20,7 +20,7 @@ use crate::types::{SpecialFormType, Type, protocol_class::ProtocolClassLiteral};
use crate::util::diagnostics::format_enumeration;
use crate::{Db, FxIndexMap, Module, ModuleName, Program, declare_lint};
use itertools::Itertools;
use ruff_db::diagnostic::{Annotation, Diagnostic, Severity, SubDiagnostic};
use ruff_db::diagnostic::{Annotation, Diagnostic, SubDiagnostic, SubDiagnosticSeverity};
use ruff_python_ast::{self as ast, AnyNodeRef};
use ruff_text_size::{Ranged, TextRange};
use rustc_hash::FxHashSet;
@ -1929,7 +1929,7 @@ pub(super) fn report_implicit_return_type(
));
let mut sub_diagnostic = SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
"Only classes that directly inherit from `typing.Protocol` \
or `typing_extensions.Protocol` are considered protocol classes",
);
@ -2037,7 +2037,7 @@ pub(crate) fn report_instance_layout_conflict(
));
let mut subdiagnostic = SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
"Two classes cannot coexist in a class's MRO if their instances \
have incompatible memory layouts",
);
@ -2230,7 +2230,7 @@ pub(crate) fn report_bad_argument_to_get_protocol_members(
diagnostic.info("Only protocol classes can be passed to `get_protocol_members`");
let mut class_def_diagnostic = SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
format_args!(
"`{}` is declared here, but it is not a protocol class:",
class.name(db)
@ -2292,7 +2292,7 @@ pub(crate) fn report_runtime_check_against_non_runtime_checkable_protocol(
diagnostic.set_primary_message("This call will raise `TypeError` at runtime");
let mut class_def_diagnostic = SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
format_args!(
"`{class_name}` is declared as a protocol class, \
but it is not declared as runtime-checkable"
@ -2326,7 +2326,7 @@ pub(crate) fn report_attempted_protocol_instantiation(
diagnostic.set_primary_message("This call will raise `TypeError` at runtime");
let mut class_def_diagnostic = SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
format_args!("Protocol classes cannot be instantiated"),
);
class_def_diagnostic.annotate(
@ -2360,7 +2360,7 @@ pub(crate) fn report_duplicate_bases(
builder.into_diagnostic(format_args!("Duplicate base class `{duplicate_name}`",));
let mut sub_diagnostic = SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
format_args!(
"The definition of class `{}` will raise `TypeError` at runtime",
class.name(db)

View file

@ -1,5 +1,5 @@
use crate::{Db, Program, PythonVersionWithSource};
use ruff_db::diagnostic::{Annotation, Diagnostic, Severity, SubDiagnostic};
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.
@ -23,7 +23,7 @@ pub fn add_inferred_python_version_hint_to_diagnostic(
crate::PythonVersionSource::ConfigFile(source) => {
if let Some(span) = source.span(db) {
let mut sub_diagnostic = SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
format_args!("Python {version} was assumed when {action}"),
);
sub_diagnostic.annotate(Annotation::primary(span).message(format_args!(
@ -39,7 +39,7 @@ pub fn add_inferred_python_version_hint_to_diagnostic(
crate::PythonVersionSource::PyvenvCfgFile(source) => {
if let Some(span) = source.span(db) {
let mut sub_diagnostic = SubDiagnostic::new(
Severity::Info,
SubDiagnosticSeverity::Info,
format_args!(
"Python {version} was assumed when {action} because of your virtual environment"
),