feat(require-input-label): add checkLabelFor option for sibling <label for> verification#2768
Draft
johanrd wants to merge 4 commits intoember-cli:masterfrom
Draft
feat(require-input-label): add checkLabelFor option for sibling <label for> verification#2768johanrd wants to merge 4 commits intoember-cli:masterfrom
johanrd wants to merge 4 commits intoember-cli:masterfrom
Conversation
…gative Refs: ember-template-lint/ember-template-lint#3388 - id + aria-label wrongly flagged as multipleLabels (false positive) - id alone not flagged as missing label (false negative)
…bel/labelledby Refs: ember-template-lint/ember-template-lint#3388 id alone cannot be verified statically as a <label for> reference (same rationale as vuejs-accessibility form-control-has-label). Counting it as a second label when aria-label or aria-labelledby is already present causes a false positive multipleLabels error. Keep the bail-out for id-only inputs (no real labels present) to avoid flagging inputs that likely have an off-screen <label for> sibling.
…el cases - Add valid: <input id aria-labelledby> (GJS + HBS) - Add invalid: <label><input id aria-label></label> still multipleLabels — locks in the corrected behavior after the validLabel && hasId carve-out was removed. - Drop peer-plugin reference from the id-skip comment; keep the spec-anchored reason (static analysis can't verify the <label for>) and inline the one-shot aria-label/labelledby locals.
…l for> verification Opt-in (default false). When enabled, an input with only a static `id` (no aria-label/labelledby/wrapping label) is verified against the set of static `<label for="X">` values collected from the same template. Inputs without a matching label are flagged. Implementation: - Single visitor pass collects `for` values into a Set - Id-only controls are deferred to Program:exit so forward-declared labels are captured (label after input) - Dynamic `id` / `for` (mustache paths, helper invocations, `(unique-id)` bindings) fall back to the existing skip behaviour — we deliberately don't resolve bindings symbolically - Set lookup keeps cross-reference O(n + m) - collectLabelFor / deferIdOnlyCheck helpers extracted to keep the GlimmerElementNode visitor under the complexity-20 lint budget Default off because Ember apps frequently split label/input across component templates (design system wrappers); enabling without that caveat would false-positive on those patterns. Documented in the rule docs.
0ee2dbd to
9236dd0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an opt-in
checkLabelForoption totemplate-require-input-labelthat verifies an<input id="X">has a matching<label for="X">in the same template — catching typos and forgotten labels that the existing rule treats as "probably labelled."Builds on #2767. The diff is layered on top of that fix and is easiest to read against it. If preferred, review both PRs together here — the underlying bug fix is the first commit in this branch's history (already approved separately in #2767).
Why opt-in (default
false)Ember apps frequently split labels and form controls across component templates (design system wrappers). Enabling this check by default would false-positive on those patterns. Teams whose templates colocate label and input can opt in.
How it works
<label for="X">values into aSetaria-label/aria-labelledby/wrapping<label>) are deferred toProgram:exit— so forward-declared labels (label after input) are capturedid={{this.fieldId}},(unique-id)bindings, helper invocations) fall back to the existing skip behaviour — we deliberately don't resolve bindings symbolicallyPrior art
angular-eslintlabel-has-associated-controlhas acheckIds: trueopt-in that takes the same two-pass approachCatches
Doesn't false-positive on
Test plan
unique-idpattern,labelTags+checkLabelForcombo, missing label, typo'dforcheckLabelForexplanation and false-positive caveat