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

Detect instance variables in all templates? #240

Open
emilong opened this issue Nov 4, 2021 · 1 comment · May be fixed by #241
Open

Detect instance variables in all templates? #240

emilong opened this issue Nov 4, 2021 · 1 comment · May be fixed by #241

Comments

@emilong
Copy link

emilong commented Nov 4, 2021

PartialInstanceVariable is great for partials, but I'd like to forbid instance variables in all templates.

The PartialInstanceVariable linter implementation has this guard to restrict it to partials:

https://github.com/Shopify/erb-lint/blob/c30fa2783d5a028b9c67795ab20f4d6b19280b9e/lib/erb_lint/linters/partial_instance_variable.rb#L11

So a linter simply without this restriction would work for my case.

The only thing between me and a PR is how to structure the configuration. Some options I came up with:

  1. Create a new InstanceVariable linter that works for all templates and document the fact that if you enable this and PartialInstanceVariable, you may get double warnings
  2. Create a new NonPartialInstanceVariable linter that only works on non-partials to avoid double linting if someone has both enabled
  3. Add configuration to PartialInstanceVariable of the sort all_templates: true to make it work for all templates
  4. Deprecate PartialInstanceVariable and create a InstanceVariable linter with a partials_only: true configuration

I'm inclined toward option 1 for simplicity, but I'm very open to whatever so I can add this linter 😄

@rafaelfranca
Copy link
Member

I prefer to go with 4.

@emilong emilong linked a pull request Nov 12, 2021 that will close this issue
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