From e7e86b85840ffc62c18d1ec2ee46740a2c9db27c Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Tue, 8 Apr 2025 12:02:06 -0400 Subject: [PATCH] red_knot_python_semantic: remove `InferContext::report_diagnostic` ... and replace it with use of `report()`. Interestingly, this is the only instance of `report_diagnostic` used directly, and thus anticipated to be the only instance of using `report()`. If this ends up being a true single use method, we could make it less generic and tailored specifically to "reveal type." Two other things to note: I left the "primary message" as empty. This avoids changing snapshots. I address this in a subsequent commit. The creation of a diagnostic here is a bit verbose/annoying. Certainly more so than it was. This is somewhat expected since our diagnostic model is more expressive and because we don't have a proc macro. I avoided creating helpers for this case since there's only one use of `report()`. But I expect to create helpers for the `lint()` case. --- .../src/types/context.rs | 39 ++++++------------- .../src/types/infer.rs | 21 +++++----- 2 files changed, 24 insertions(+), 36 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types/context.rs b/crates/red_knot_python_semantic/src/types/context.rs index a5ca632f57..764554c84d 100644 --- a/crates/red_knot_python_semantic/src/types/context.rs +++ b/crates/red_knot_python_semantic/src/types/context.rs @@ -57,6 +57,16 @@ impl<'db> InferContext<'db> { self.file } + /// Create a span with the range of the given expression + /// in the file being currently type checked. + /// + /// If you're creating a diagnostic with snippets in files + /// other than this one, you should create the span directly + /// and not use this convenience API. + pub(crate) fn span(&self, ranged: T) -> Span { + Span::from(self.file()).with_range(ranged.range()) + } + pub(crate) fn db(&self) -> &'db dyn Db { self.db } @@ -87,7 +97,7 @@ impl<'db> InferContext<'db> { ) where T: Ranged, { - let Some(mut reporter) = self.lint(lint, "") else { + let Some(builder) = self.lint(lint) else { return; }; let mut reporter = builder.build(""); @@ -99,32 +109,7 @@ impl<'db> InferContext<'db> { diag.annotate(Annotation::primary(span).message(message)); } - /// Adds a new diagnostic. - /// - /// The diagnostic does not get added if the rule isn't enabled for this file. - pub(super) fn report_diagnostic( - &self, - ranged: T, - id: DiagnosticId, - severity: Severity, - message: fmt::Arguments, - secondary_messages: &[OldSecondaryDiagnosticMessage], - ) where - T: Ranged, - { - let Some(mut reporter) = self.report(id, severity, "") else { - return; - }; - let mut reporter = builder.build(""); - let diag = reporter.diagnostic(); - for secondary_msg in secondary_messages { - diag.sub(secondary_msg.to_sub_diagnostic()); - } - let span = Span::from(self.file).with_range(ranged.range()); - diag.annotate(Annotation::primary(span).message(message)); - } - - /// Optionally return a reporter builder for adding a lint diagnostic. + /// Optionally return a reporter for adding a lint diagnostic. /// /// If the current context believes a diagnostic should be reported for /// the given lint, then a reporter is returned that enables building up a diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index ed64d04c5e..a15ece508b 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -34,7 +34,7 @@ //! of iterations, so if we fail to converge, Salsa will eventually panic. (This should of course //! be considered a bug.) use itertools::{Either, Itertools}; -use ruff_db::diagnostic::{DiagnosticId, Severity}; +use ruff_db::diagnostic::{Annotation, DiagnosticId, Severity}; use ruff_db::files::File; use ruff_db::parsed::parsed_module; use ruff_python_ast::visitor::{walk_expr, Visitor}; @@ -4105,16 +4105,19 @@ impl<'db> TypeInferenceBuilder<'db> { match known_function { KnownFunction::RevealType => { if let [Some(revealed_type)] = overload.parameter_types() { - self.context.report_diagnostic( - call_expression, + if let Some(mut reporter) = self.context.report( DiagnosticId::RevealedType, Severity::Info, - format_args!( - "Revealed type is `{}`", - revealed_type.display(self.db()) - ), - &[], - ); + "", + ) { + let span = self.context.span(call_expression); + reporter.diagnostic().annotate( + Annotation::primary(span).message(format_args!( + "Revealed type is `{}`", + revealed_type.display(self.db()) + )), + ); + } } } KnownFunction::AssertType => {