Convert OldDiagnostic::noqa_code to an Option<String>

[ntBre]

Summary

I think this should be the last step before combining OldDiagnostic and ruff_db::Diagnostic. We can't store a NoqaCode on ruff_db::Diagnostic, so I converted the noqa_code field to an Option<String> and then propagated this change to all of the callers.

I tried to use &str everywhere it was possible, so I think the remaining to_string calls are necessary. I spent some time trying to convert everything to &str but ran into lifetime issues, especially in the FixTable. Maybe we can take another look at that if it causes a performance regression, but hopefully these paths aren't too hot. We also avoid some to_string calls, so it might even out a bit too.

Test Plan

Existing tests

[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.

[ntBre]

The performance is a bit worse (-2% in the worst case) on the all-rules benchmarks, but it seems widely dispersed, so I think it may just be from the up-front NoqaCode::to_string calls for storing them in OldDiagnostic, not the later conversions.

[MichaReiser]

This mostly looks good. I think we want to keep using Rule in noqa.rs. But I'm not sure if that's better done in a separate PR

I also suggest introducing a SecondaryCode struct. It's hard to tell what all the &str and String usages are (especially because there are so few comments)

[MichaReiser]

I wonder if we should add a SecondaryCode struct which wraps a String I do find the &str makes the code harder to understand.

[ntBre]

Hmm, I think if it wrapped a String we'd have to clone in OldDiagnostic::secondary_code right? I'll try it with a &str and see if we can use that everywhere. I definitely agree that it would be more readable, though.

[ntBre]

I think I was responding to these too early in the morning 🤦 I can store the SecondaryCode(String) on the OldDiagnostic and then use &SecondaryCode where I'm currently using &str. I'm working on that, and then I'll try the Rule changes on a separate branch!

[github-actions]

mypy_primer results

No ecosystem changes detected ✅