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

ThisAssembly.Strings: If resource name is not a valid code identifier, compilation fails #83

Closed
kzu opened this issue Nov 30, 2021 · 6 comments · Fixed by #249
Closed
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@kzu
Copy link
Member

kzu commented Nov 30, 2021

If a resx file contains an entry with an invalid code identifier (like a number), the generated code will not compile, and the error is quite cryptic too.

We should validate that the name are valid C# identifiers and skip (probably warn too if they aren't?)

@kzu kzu added the bug Something isn't working label Nov 30, 2021
@rdeago
Copy link

rdeago commented May 19, 2022

Some quick notes:

  • The same check may be useful in other situations where identifiers are involved, e.g. user-defined constants.
  • Roslyn has a handy IsValidIdentifier utility method that can be used for this purpose.
  • There's an IsValidIdentifier method for Visual Basic too, so you may call one or the other according to the project's language.
  • F# is no concern, as it doesn't support code generators.

@kzu kzu added the good first issue Good for newcomers label Jul 20, 2022
@kzu
Copy link
Member Author

kzu commented Jan 10, 2023

I guess more than just checking whether the identifier is valid, you'd actually want it to generate code but turn the identifier into a valid one by replacing invalid chars with underscores, say...

@kzu
Copy link
Member Author

kzu commented Jan 10, 2023

@viceroypenguin perhaps this could also use the sanitization you applied to resource class names?

@viceroypenguin
Copy link
Contributor

Yeah, I encountered this while working on Resources; I just got lazy and avoided it for now... 🙂. I would expect the code to work the same here. Could copy/paste or extract to common .cs file. The regex is doing the bulk of the work, so copy/paste would probably be fine.

@PhenX
Copy link
Contributor

PhenX commented Jul 28, 2023

This method could be used to generate a valid identifier : it replaces any invalid identifier by '', removes redundant '', and prefixes the identifier by '_' if not valid :

        private string EscapeIdentifier(string filename)
        {
            var bytes = filename.ToCharArray();

            var replaced = bytes.Select(_ => SyntaxFacts.IsIdentifierPartCharacter(_) ? _ : '_').ToArray();

            var result = Regex.Replace(new string(replaced), "(_)+", "_");

            if (!SyntaxFacts.IsValidIdentifier(result))
            {
                result = "_" + result;
            }

            return result;
        }

As for now, all we can do is change all the files' name, but that's not always easy.

@kzu
Copy link
Member Author

kzu commented Aug 2, 2023

Thanks for the suggestion @PhenX. Would you like to send a PR? 🙏

@kzu kzu closed this as completed in #249 Aug 30, 2023
@devlooped devlooped locked and limited conversation to collaborators Sep 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants