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 custom linter rule to prevent usage of string literals in widgets #260

Open
bdmendes opened this issue Mar 22, 2024 · 8 comments · May be fixed by #261
Open

Add custom linter rule to prevent usage of string literals in widgets #260

bdmendes opened this issue Mar 22, 2024 · 8 comments · May be fixed by #261

Comments

@bdmendes
Copy link

When using internalization (e.g. through a package such as i18n), no text placed in the UI should be hardcoded to one language. We could implement a custom linter rule to prevent this from happening.

@shilangyu
Copy link
Contributor

This should be easy to implement, but I fear about all the false positives this might raise. This would have to be empirically verified. Where exactly do you think string literals should be disallowed? In the build method of widgets? In the whole widget class?

@bdmendes
Copy link
Author

bdmendes commented Mar 22, 2024

I was thinking in the build method. We might have a issue about logs though, how would you approach that?
(Would functions returning widgets called in the build method be explored too?)

@shilangyu
Copy link
Contributor

Would functions returning widgets called in the build method be explored too?

It all depends on what logic we would define for this.

We might have a issue about logs though, how would you approach that

There could be some config to describe exceptions for where the lint error should be raised.

TBH I feel like there would be a plethora of false positives making this lint very annoying to use.

@bdmendes
Copy link
Author

I'll propose an implementation in the next hours and perhaps we can discuss it better looking at the code.
From an outsider perspective, I would suggest that contributions are better documented, since it's quite hard to understand how to run tests for example

@shilangyu
Copy link
Contributor

Thanks for the suggestion, its true that the codebase is not prepared for outside contributions.

Regarding the implementation, before you jump into code I think we can first discuss what kind of logic we want here, since implementation will be the least of the problems methinks.

@bdmendes
Copy link
Author

bdmendes commented Mar 22, 2024

Sure, I'm experimenting so we can use it on our project directly using my fork (or simply putting the code in our project) if the team wants it.
I agree that to be included here, we should carefully consider the exceptions.
Ideally, if we could catch only strings that are passed as parameters to widgets, that would be great. However, I'm not sure that is possible. For instance, if I use the literal to initialize a variable and pass the variable to the widget constructor, would it count as a string literal there?

@bdmendes
Copy link
Author

Here's a quick proposal draft to further add to our discussion: #261
I'll add @bartekpacia to the discussion.

@shilangyu
Copy link
Contributor

For instance, if I use the literal to initialize a variable and pass the variable to the widget constructor

This is definitely possible to track. Generally our approach to lints is that we are a-ok with them not catching every violation, but we want to avoid false-positives to fullest extent.

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 a pull request may close this issue.

2 participants