Skip to content

Linter: Implement html-no-nested-forms rule - #1961

Open
irinanazarova wants to merge 1 commit into
marcoroth:mainfrom
irinanazarova:html-no-nested-forms
Open

Linter: Implement html-no-nested-forms rule#1961
irinanazarova wants to merge 1 commit into
marcoroth:mainfrom
irinanazarova:html-no-nested-forms

Conversation

@irinanazarova

Copy link
Copy Markdown

This pull request implements a new html-no-nested-forms rule that disallows placing one <form> element inside another <form> element, including forms rendered by Action View helpers.

<%= form_with model: @mission do |form| %>
  <%= form.submit "Update" %>
  <%= button_to "Delete", mission_path(@mission), method: :delete %>
<% end %>
`button_to` renders its own `<form>` element and cannot be nested inside another form. Move it outside of the enclosing form.

HTML does not support nested forms: browsers silently drop or re-parent the inner form, leading to broken submissions and buttons that submit the wrong form. The helper case is particularly easy to miss because the nested <form> never appears in the template. In the example above, the form rendered by button_to is dropped during parsing and clicking "Delete" submits the outer form instead, performing an update.

The rule detects literal <form> elements with a depth counter (same approach as html-no-nested-links) and form-rendering helpers via Prism (same approach as actionview-no-silent-helper). The helper list comes from the Action View helper registry via getHelpersForTag("form"), filtered to helpers the parser does not transform yet: once a form helper becomes supported, the element branch catches its synthesized <form> element and the Prism branch stops covering it automatically. button_to is included per the discussion in #166, since it is effectively a hidden form. The offense is reported on the inner form's tag name, or on the helper call for helper-rendered forms.

Nesting composed across partials (an outer form in one template, an inner form in a rendered partial) is not detectable with per-file linting. tag.form and content_tag(:form) are not detected yet; the registry's detectStyle metadata encodes how to find these. This could be a follow-up.

Resolves #166

@github-actions github-actions Bot added documentation Improvements or additions to documentation linter typescript linter-rule labels Aug 2, 2026

@marcoroth marcoroth left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks @irinanazarova, this looks great!

Just a few nits and this is good to go, thank you!


if (this.formDepth > 0) {
this.addOffense(
"Nested `<form>` elements are not allowed. Move the inner `<form>` outside of the enclosing form, or associate its controls using the `form` attribute.",

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
"Nested `<form>` elements are not allowed. Move the inner `<form>` outside of the enclosing form, or associate its controls using the `form` attribute.",
"Nested `<form>` elements are not allowed. Move the inner `<form>` outside of the enclosing `<form>`, or associate its controls using the `form` attribute.",

if (this.formDepth === 0) return

this.addOffense(
`\`${helperName}\` renders its own \`<form>\` element and cannot be nested inside another form. Move it outside of the enclosing form.`,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
`\`${helperName}\` renders its own \`<form>\` element and cannot be nested inside another form. Move it outside of the enclosing form.`,
`\`${helperName}\` renders its own \`<form>\` element and cannot be nested inside another \`<form>\`. Move it outside of the enclosing \`<form>\`.`,

this.formDepth--
}

visitERBBlockNode(node: ERBBlockNode): void {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
visitERBBlockNode(node: ERBBlockNode): void {
// TODO: remove once we support transforming all the Action View form helpers
visitERBBlockNode(node: ERBBlockNode): void {

this.formDepth--
}

visitERBContentNode(node: ERBContentNode): void {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
visitERBContentNode(node: ERBContentNode): void {
// TODO: remove once we support transforming all the Action View form helpers
visitERBContentNode(node: ERBContentNode): void {

assertOffenses(dedent`
<form><div><form></form></div></form>
`)
})

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

lets also add tests for form_tag

@marcoroth marcoroth added this to the v0.11.0 milestone Aug 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation linter linter-rule typescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Linter Rule: Disallow nested forms

2 participants