ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

The PR summary is a bit confusing because it isn't clear to me when/if you're speaking about the ruff or linter Diagnostic type. Maybe rewrite it to DiagnosticOld and clarify at the beginning what Diagnostic points to.

(It may be too late for this but I liked what @BurntSushi did early on in his refactor: He renamed the old Diagnostic type to OldDiagnostic. This helped clarify a lot in his PRs)

Oh sorry, this PR is all about the old ruff_diagnostics::Diagnostic. I guess I only typed that out once and then dropped the prefix. The new ruff_db::Diagnostics also have a file attached, so the old ruff_diagnostics::Diagnostic type is the only one of the three (including Message) without an associated file.

I think you might be right about it being a bit late. My plan after this and #18051 is to delete ruff_diagnostics::Diagnostic (OldDiagnostic) entirely. I had a branch stacked on #18051 doing that when I realized I needed the file, which seemed good to split off.

I feel bad saying this because this must have taken you a lot of time but I feel a bit concerned about introducing a new required argument for every lint rule.

Could we take inspiration from InferContext::report_lint and e.g. introduce a checker.report_violation(violation, range) that returns a DiagnosticGuard that automatically injects the file and pushes the diagnostic on drop?

I didn't review all call sites to get a good sense if this works in all cases but it would avoid passing file everywhere

No worries, I was concerned about that too. I'll take a look at InferContext::report_lint!

It definitely won't work everywhere, at least as a method on Checker, because we don't have a Checker available in a surprising number of cases. However, we still can pass a Checker to some subset of those. That's actually what I did first before switching to pass only the SourceFile. So I think it will still be useful in the vast majority of cases.

It definitely won't work everywhere, at least as a method on Checker, because we don't have a Checker available in a surprising number of cases. However, we still can pass a Checker to some subset of those. That's actually what I did first before switching to pass only the SourceFile. So I think it will still be useful in the vast majority of cases.

I'm fine passing SourceFile in places where we don't have a Checker. I think some of those have a context parameter on which we can store it instead.

I'm going to go ahead and close this one instead of trying to rebase onto #18051. I'll put up a second attempt with a new Checker method for constructing diagnostics after I study InferContext a bit.