Skip to content

Linter: Implement html-input-require-autocomplete linter rule #565

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.

Sign up for GitHub

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

Jump to bottom
Merged
marcoroth merged 10 commits into from
Oct 14, 2025
Merged

Linter: Implement html-input-require-autocomplete linter rule #565

marcoroth merged 10 commits into from
Oct 14, 2025

Conversation

@domingo2000
Copy link
Contributor

@domingo2000 domingo2000 commented Oct 1, 2025 •
edited by marcoroth
Loading

@domingo2000
Copy link
Contributor Author

domingo2000 commented Oct 1, 2025

Currently I only implemented the rule for the HTML nodes.

For the ruby CallNodes that are inside of the content of ERBNodes i don't know how to proceed, I checked the other rules, but there is no Ruby Prism AST inside the ERB nodes to analyze with visitors.

I think this could be done in 2 ways.

  1. Make the mixes AST nodes with the prsim nodes inside the ERB tags available to the linter: This would be nice and very flexible for other rules, making very similar to lint ruby nodes inside ERB nodes easily.
  2. With the rubocop integration, we could write a rubocop rule that lint those helpers in rubocop.

@marcoroth if you have any more ideas in how to cover the remaining tests for the form helpers, I would be happy to help.

@domingo2000 domingo2000 force-pushed the add-require-input-autocomplete branch from e1015fa to e51e814 Compare October 1, 2025 16:55
@domingo2000 domingo2000 changed the title Linter: Implement linter rule (#552) Linter: Implement RequireInputAutocomplete linter rule (#552) Oct 1, 2025
@domingo2000 domingo2000 force-pushed the add-require-input-autocomplete branch from e51e814 to 8e4ee3e Compare October 1, 2025 16:56
@marcoroth marcoroth changed the title Linter: Implement RequireInputAutocomplete linter rule (#552) Linter: Implement html-inputs-require-autocomplete linter rule Oct 5, 2025
marcoroth
marcoroth approved these changes Oct 14, 2025
View reviewed changes
Copy link
Owner

@marcoroth marcoroth left a comment

Choose a reason for hiding this comment

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

Thanks @domingo2000!

Currently I only implemented the rule for the HTML nodes.

This is fine, all rules currently have this limitation. We are going to solve this in the parser directly, so that the rules can always assume to operate on HTML elements/the Herb Syntax Tree, even if the source defines the HTML elements in ERB helpers.

I touched up the docs/messages a bit and migrated to the new tests. So far so good, thanks for working on this @domingo2000! 🙏🏼

@marcoroth marcoroth changed the title Linter: Implement html-inputs-require-autocomplete linter rule Linter: Implement html-input-require-autocomplete linter rule Oct 14, 2025
@marcoroth marcoroth merged commit 62dd9a8 into marcoroth:main Oct 14, 2025
1 check passed
@asilano asilano mentioned this pull request Oct 21, 2025
Merged
asilano pushed a commit to fac/herb that referenced this pull request Oct 21, 2025
@domingo2000 @marcoroth @asilano
Linter: Implement html-input-require-autocomplete linter rule (marc…
cd13bf7 
…oroth#565)

Resolves marcoroth#552

---------

Co-authored-by: Marco Roth <marco.roth@intergga.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@marcoroth marcoroth marcoroth approved these changes

Assignees

No one assigned

Labels

erb-lint linter linter-rule

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

erb-lint: RequireInputAutocomplete

2 participants

@domingo2000 @marcoroth