Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add analyser which checks uniqueness of translation keys in a single file #63

Merged
merged 3 commits into from
May 17, 2024

Conversation

bdach
Copy link
Contributor

@bdach bdach commented May 16, 2024

The goal here is to catch cases like ppy/osu#27472 and ppy/osu#28190 in an automated manner.

Have confirmed that on ppy/osu@f9fd1b9 this produces the following build errors:

/home/dachb/Documents/opensource/osu/osu.Game/Localisation/DeleteConfirmationContentStrings.cs(30,86): error OLOC004: The localisation key 'collections' has been used multiple times in this class [/home/dachb/Documents/opensource/osu/osu.Game/osu.Game.csproj]
/home/dachb/Documents/opensource/osu/osu.Game/Localisation/DeleteConfirmationContentStrings.cs(35,81): error OLOC004: The localisation key 'collections' has been used multiple times in this class [/home/dachb/Documents/opensource/osu/osu.Game/osu.Game.csproj]

and no others.

namespace LocalisationAnalyser.Analysers
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class LocalisationKeyUsedMultipleTimesInClassAnalyser : DiagnosticAnalyzer
Copy link
Contributor Author

@bdach bdach May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This purposefully does not inherit AbstractMemberAnalyser. I wanted to run the duplicate check exactly once per file, and I don't trust that any other method of passing data inside an analyser other than the stack is safe (as I have no idea what guarantees - if any - there are wrt threading and/or analyser lifetime, and there are indications online that there aren't any to guarantee that anything other than the stack is safe), so in order to not pollute AbstractMemberAnalyser with things specific to this check, I decided to just do away with inheritance and duplicate the few lines of logic it takes to reimplement it from scratch.

Copy link
Collaborator

@smoogipoo smoogipoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@smoogipoo smoogipoo merged commit 07256c6 into ppy:master May 17, 2024
7 checks passed
@bdach bdach deleted the check-localisation-keys-for-uniqueness branch May 17, 2024 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants