Document when a rule was added

[ntBre]

Summary

Inspired by #20859, this PR adds the version a rule was added, and the file and line where it was defined, to ViolationMetadata. The file and line just use the standard file! and line! macros, while the more interesting version field uses a new violation_metadata attribute parsed by our ViolationMetadata derive macro.

I moved the commit modifying all of the rule files to the end, so it should be a lot easier to review by omitting that one.

As a curiosity and a bit of a sanity check, I also plotted the rule numbers over time:

I think this looks pretty reasonable and avoids some of the artifacts the earlier versions of the script ran into, such as the rule sub-command not being available or --explain requiring a file argument.

Script and summary data

gawk --csv '
NR > 1 {
    split($2, a, ".")
    major = a[1]; minor = a[2]; micro = a[3]
    # sum the number of rules added per minor version
    versions[minor] += 1
}
END {
    tot = 0
    for (i = 0; i <= 14; i++) {
        tot += versions[i]
        print i, tot
    }
}
' ruff_rules_metadata.csv > summary.dat

0 696
1 768
2 778
3 803
4 822
5 848
6 855
7 865
8 893
9 915
10 916
11 924
12 929
13 932
14 933

Test Plan

I built and viewed the documentation locally, and it looks pretty good!

The spacing seems a bit awkward following the h1 at the top, so I'm wondering if this might look nicer as a footer in Ruff. The links work well too:

• v0.0.271
• Related issues
• View source

The last one even works on main now since it points to the derive(ViolationMetadata) line.

In terms of binary size, this branch is a bit bigger than main with 38,654,520 bytes compared to 38,635,728 (+20 KB). I guess that's not too much of an increase, but I wanted to check since we're generating a lot more code with macros.

[github-actions]

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.

[MeGaGiGaGon]

That's fun, I wasn't aware ruff rule existed, that would have made my other two previous adventures with iterating over all the rules much easier. I'll have to go back at some point and update my scripts for those, since I may have missed a couple rules while regexing my way through the problems.

Related to that, this may be a stupid question, but will the changes in this PR be reflected in ruff rule? (As in, after this will ruff rule --all --output-format json also include items for version, file, and line?) That would be even more convenient for my purposes, since I wouldn't need to do any of the regex spaghetti to get rule definition location info in the first place.

[MichaReiser]

Nice! That's what you were up to yesterday :)

ViolationMetadata seems the right spot for file and location but I'm less sure about the version. Mainly because the rule status is now spread over two places:

• code_to_rule: Defines the status of the rule (preview, removed, stable)
• violation_metadata: Defines the version.

This has the downside that It's unclear what the version represents. Is it when the rule was added, deprecated or removed?

That's why I'm inclined to combine the version and the RuleStatus, similar to LintStatus in ty. Whether we simply extend the RuleStatus variants to have extra fields or move RuleStatus to ViolationMetadata (that's probably what I'd do if I would do it anew) isn't important and I'd probably go with what's easier.

[ntBre]

Related to that, this may be a stupid question, but will the changes in this PR be reflected in ruff rule? (As in, after this will ruff rule --all --output-format json also include items for version, file, and line?) That would be even more convenient for my purposes, since I wouldn't need to do any of the regex spaghetti to get rule definition location info in the first place.

@MeGaGiGaGon I think that's a great question and a good idea to add. It crossed my mind early on but then I kind of forgot about it. I think it should be straightforward to add here.

@MichaReiser that makes a lot of sense. I did like how LintStatus looked in ty. I'll try to see how it looks to move RuleGroup into the new macro attribute this morning, but I think adding the version as a field is probably easier.

[ntBre]

As Micha and I discussed on Discord, the RuleGroup and version information have now been combined, more like LintStatus in ty. This is an example usage of the new attribute:

#[derive(ViolationMetadata)]
#[violation_metadata(stable_since = "v0.0.155")]
pub(crate) struct UnusedNOQA {
    pub codes: Option<UnusedCodes>,
}

One interesting and somewhat intentional aspect of the attribute is that the macro will preserve the last entry, so we could theoretically stack versions if the status of a rule changes over time. For example, this would be accepted by the current macro implementation but only use the removed_since field:

#[derive(ViolationMetadata)]
#[violation_metadata(
    preview_since = "v0.0.123", 
    stable_since = "v0.0.155", 
    deprecated_since = "v1.2.3", 
    removed_since = "v4.5.6",
)]
pub(crate) struct UnusedNOQA {
    pub codes: Option<UnusedCodes>,
}

Another alternative we considered was just using the RuleGroup variants directly, like:

#[derive(ViolationMetadata)]
#[violation_metadata(status = RuleGroup::Stable { since: "v0.0.155" })]
pub(crate) struct UnusedNOQA {
    pub codes: Option<UnusedCodes>,
}

which is also pretty nice. If we ever decide to add a reason field like ty has for the deprecated and removed variants this might be even more compelling. Or we could just add a reason field to the macro too.

---

I also updated the JSON output format for ruff rule to include the new file and line information (the version was added automatically by including it in RuleGroup).

And I rebased a bit more to keep the big commits at the end. The last commit is still add metadata to rules and the second-to-last is the big edit to remove the RuleGroup entries from the map_codes table.

[MichaReiser]

This works for me. I think having status = might be slightly easier to discover as you can type RuleStatus:: to see all the variants that are supported. But I don't feel strongly about the syntax (having all versions stacked looks a bit nosiy and having multiple fields might be easier to parse).

[ntBre]

Sounds good! I guess I'll go ahead and land this once I fix the doctests and remove the scripts and CSV since it should be easy to iterate on the syntax now that we have the infrastructure in place.

Maybe this is just my editor, but I wasn't getting completions when I tried typing RuleGroup:: in the attribute. But otherwise I like that fine too, no strong feelings on the exact syntax.

[ntBre]

One last check to make sure my scripts didn't change any of the rule statuses:

diff \
    <(./target/debug/ruff rule --all --output-format json | jq '[ .[] | { code, status: ( .status | keys[0] ) } ]') \
    <(../worktrees/ruff1/target/debug/ruff rule --all --output-format json | jq '[ .[] | { code, status} ]')

passes cleanly with main checked out in my other worktree.

[ntBre]

Oh, I just realized that many of the versions are actually wrong after the RuleGroup change... I collected all of the Added in versions but now they need to be when they were stabilized or removed for many rules instead.

[ntBre]

All of the removed_since and stable_since versions are updated. Preview should have already been correct, and we don't currently have any deprecated rules. I'll merge once CI finishes and then start the release for the day. Thanks again!