Do not use code location for Gitlab fingerprints.

[gregersn]

Using code location when creating hash for Gitlab fingerprints makes the codequality reports in Gitlab very unstable. Any change of position would trigger both a "fixed" message, and a "new problem detected" message in Gitlab, making it very noisy.

Summary

Do not use code location for Gitlab fingerprints.

Using code location when creating hash for Gitlab fingerprints
makes the codequality reports in Gitlab very unstable.
Any change of position would trigger both a "fixed" message, and
a "new problem detected" message in Gitlab, making it very noisy.

Test Plan

By running RUFF_UPDATE_SCHEMA=1 cargo test

Also, ran cargo run -p ruff_cli -- /path/to/pyfile --no-cache --format gitlab before and after moving adding new lines, and observed that fingerprints in report did not change.

Close #7159

[github-actions]

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

[zanieb]

A comment about the purpose of this may be useful.

Doesn't this make the fingerprint change based on previous fingerprints? Won't this dependency change a downstream fingerprint if a conflicting upstream diagnostic is fixed?

[gregersn]

My thinking is that "value for money", just doing this will be a lot less annoying and give a lot less noise in the gitlab reports, than having code position included, which would trigger for almost no reason. At least with this method the false reporting of fixed/new issues will either be either when a new issue is introduced, which has the same name, or an issue which there are multiples are with the same name, is fixed.

[MichaReiser]

The main "issue" I see with the heuristic is that GitLab will mark the wrong diagnostic as fixed if there's a hash collision, but I haven't developed a better heuristic.

Let's say we have two unused variable diagnostics in a file:

x = 1
y = 2

There's a hash collision for x and y, so y performs a second hashing round

and we now fix the first violation

x = 1
y = 2
print(x)

There's no longer a hash collision for y, meaning that the diagnostic for y now gets the hash (fingerprint) of the violation that used to be for x.

We could consider including more information when a hash collision occurs but not sure what information that would be that doesn't suffer from the original issue and distinguishes multiple diagnostics sufficiently.

[MichaReiser]

Thank you so much for your contribution. I hope you had some fun writing your first Rust code.

I ask you to change fingerprint to return a u64 and delay the string serialization until we initialize the "fingerprint" field for improved performance and lower memory usage.

We're then good to mere and pack this into the next release!

[MichaReiser]

This is unrelated to your change but I noticed it now when reviewing this PR. It would probably be better to hash kind.name rather than the rule because the Rule's hash implementation changes every time a new rule is inserted into the Rule enum above which happens frequently. What do you think @zanieb ?

Suggested change

	kind.rule().hash(&mut hasher);
	kind.name.hash(&mut hasher);

[gregersn]

This made sense to change, as you present it, since that could lead to as much a breaking change every time a new rule is added, as changing the hashing as we are doing with this.