fix(html/useAltText): handle Vue dynamic :alt and v-bind:alt bindings#9355
Conversation
🦋 Changeset detectedLatest commit: 8cefcc2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
A-Linter
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR updates the useAltText accessibility lint to treat Vue dynamic alt bindings as valid by changing its check to call Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dyc3
left a comment
dyc3
left a comment
There was a problem hiding this comment.
- needs a changeset
- this isn't the first time a bug like this happened. we should add a generic function in
biome_html_syntaxthat can extract an attribute or a vue binding based on the the attribute name its targeting.
| arg.syntax() | ||
| .text_trimmed() | ||
| .to_string() | ||
| .trim_start_matches(':') | ||
| .to_string() | ||
| } |
There was a problem hiding this comment.
.to_string()allocates strings on the heap, and we shouldn't need to do that for this check. Use borrowed strings, or if you really need an owned string, useTokenText.- you also shouldn't need to trim
:, you can simply check the static directive argument.
There was a problem hiding this comment.
No more .to_string() or : trimming, using as_vue_static_argument() → name_token() directly
| .to_string() | ||
| } | ||
|
|
||
| let is_alt = |attrs: &biome_html_syntax::HtmlAttributeList| { |
There was a problem hiding this comment.
nit: this shouldn't need to be a closure
There was a problem hiding this comment.
Moved the logic to has_attribute_or_vue_binding on AnyHtmlElement in element_ext.rs — no more closure
|
@dyc3 added the changeset and addressed all review comments. Should I split the has_attribute_or_vue_binding into two separate functions — one for plain HTML attributes and one for Vue bindings — or is the combined function preferred for now? |
| /// Handles: | ||
| /// - `name="..."` — standard HTML attribute | ||
| /// - `:name="..."` — Vue v-bind shorthand (`VueVBindShorthandDirective`) | ||
| /// - `v-bind:name="..."` — explicit Vue v-bind (`VueDirective`) | ||
| pub fn has_attribute_or_vue_binding(&self, name_to_lookup: &str) -> bool { |
There was a problem hiding this comment.
This should return an Option<AnyHtmlAttribute> with the found attribute. It would give the function more utility.
|
The combined function is preferred, thank you. |
SchahinRohani
force-pushed
the
fix/use-alt-text-vue-dynamic-binding
branch
from
6beb0ae to
f4b63e5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_html_syntax/src/element_ext.rs`:
- Around line 148-165: The matching logic in
AnyVueDirective::VueVBindShorthandDirective and the
AnyVueDirective::VueDirective branch must strip a leading ':' from the argument
token before comparing to name_to_lookup and make the v-bind name check
ASCII-case-insensitive; update the arg name extraction (used in the chains that
call name_token().ok() and text_trimmed()) to obtain the token text, remove a
leading ':' (e.g. trim_start_matches(':')) and then compare with
eq_ignore_ascii_case(name_to_lookup), and change the v-bind detection (currently
t.text_trimmed() == "v-bind") to use eq_ignore_ascii_case so both branches
correctly recognise :alt, v-bind:alt and V-BIND:alt forms.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: abe3b9aa-1746-497e-9f84-16317c72d715
⛔ Files ignored due to path filters (1)
crates/biome_html_analyze/tests/specs/a11y/useAltText/vue/valid.vue.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (2)
crates/biome_html_analyze/src/lint/a11y/use_alt_text.rscrates/biome_html_syntax/src/element_ext.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_html_analyze/src/lint/a11y/use_alt_text.rs
| AnyVueDirective::VueVBindShorthandDirective(d) => d | ||
| .arg() | ||
| .ok() | ||
| .and_then(|arg| arg.arg().ok()) | ||
| .and_then(|arg| arg.as_vue_static_argument().cloned()) | ||
| .and_then(|s| s.name_token().ok()) | ||
| .is_some_and(|t| t.text_trimmed().eq_ignore_ascii_case(name_to_lookup)), | ||
|
|
||
| // v-bind:name="..." | ||
| AnyVueDirective::VueDirective(d) => { | ||
| let is_bind = d.name_token().is_ok_and(|t| t.text_trimmed() == "v-bind"); | ||
| is_bind | ||
| && d.arg() | ||
| .and_then(|arg| arg.arg().ok()) | ||
| .and_then(|arg| arg.as_vue_static_argument().cloned()) | ||
| .and_then(|s| s.name_token().ok()) | ||
| .is_some_and(|t| { | ||
| t.text_trimmed().eq_ignore_ascii_case(name_to_lookup) |
There was a problem hiding this comment.
Strip the leading : before matching Vue arg names.
These branches still compare the raw directive argument token against "alt". Per the parser shape described in this PR, that token is ":alt", so the helper can still miss the two Vue forms it is meant to recognise. I’d also make the v-bind check ASCII-case-insensitive for parity with normal attribute matching — false positives are a bit less funny when they survive the fix.
Proposed fix
AnyVueDirective::VueVBindShorthandDirective(d) => d
.arg()
.ok()
.and_then(|arg| arg.arg().ok())
.and_then(|arg| arg.as_vue_static_argument().cloned())
.and_then(|s| s.name_token().ok())
- .is_some_and(|t| t.text_trimmed().eq_ignore_ascii_case(name_to_lookup)),
+ .is_some_and(|t| {
+ let name = t.text_trimmed();
+ name.strip_prefix(':')
+ .unwrap_or(name)
+ .eq_ignore_ascii_case(name_to_lookup)
+ }),
// v-bind:name="..."
AnyVueDirective::VueDirective(d) => {
- let is_bind = d.name_token().is_ok_and(|t| t.text_trimmed() == "v-bind");
+ let is_bind = d
+ .name_token()
+ .is_ok_and(|t| t.text_trimmed().eq_ignore_ascii_case("v-bind"));
is_bind
&& d.arg()
.and_then(|arg| arg.arg().ok())
.and_then(|arg| arg.as_vue_static_argument().cloned())
.and_then(|s| s.name_token().ok())
.is_some_and(|t| {
- t.text_trimmed().eq_ignore_ascii_case(name_to_lookup)
+ let name = t.text_trimmed();
+ name.strip_prefix(':')
+ .unwrap_or(name)
+ .eq_ignore_ascii_case(name_to_lookup)
})
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| AnyVueDirective::VueVBindShorthandDirective(d) => d | |
| .arg() | |
| .ok() | |
| .and_then(|arg| arg.arg().ok()) | |
| .and_then(|arg| arg.as_vue_static_argument().cloned()) | |
| .and_then(|s| s.name_token().ok()) | |
| .is_some_and(|t| t.text_trimmed().eq_ignore_ascii_case(name_to_lookup)), | |
| // v-bind:name="..." | |
| AnyVueDirective::VueDirective(d) => { | |
| let is_bind = d.name_token().is_ok_and(|t| t.text_trimmed() == "v-bind"); | |
| is_bind | |
| && d.arg() | |
| .and_then(|arg| arg.arg().ok()) | |
| .and_then(|arg| arg.as_vue_static_argument().cloned()) | |
| .and_then(|s| s.name_token().ok()) | |
| .is_some_and(|t| { | |
| t.text_trimmed().eq_ignore_ascii_case(name_to_lookup) | |
| AnyVueDirective::VueVBindShorthandDirective(d) => d | |
| .arg() | |
| .ok() | |
| .and_then(|arg| arg.arg().ok()) | |
| .and_then(|arg| arg.as_vue_static_argument().cloned()) | |
| .and_then(|s| s.name_token().ok()) | |
| .is_some_and(|t| { | |
| let name = t.text_trimmed(); | |
| name.strip_prefix(':') | |
| .unwrap_or(name) | |
| .eq_ignore_ascii_case(name_to_lookup) | |
| }), | |
| // v-bind:name="..." | |
| AnyVueDirective::VueDirective(d) => { | |
| let is_bind = d | |
| .name_token() | |
| .is_ok_and(|t| t.text_trimmed().eq_ignore_ascii_case("v-bind")); | |
| is_bind | |
| && d.arg() | |
| .and_then(|arg| arg.arg().ok()) | |
| .and_then(|arg| arg.as_vue_static_argument().cloned()) | |
| .and_then(|s| s.name_token().ok()) | |
| .is_some_and(|t| { | |
| let name = t.text_trimmed(); | |
| name.strip_prefix(':') | |
| .unwrap_or(name) | |
| .eq_ignore_ascii_case(name_to_lookup) | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_html_syntax/src/element_ext.rs` around lines 148 - 165, The
matching logic in AnyVueDirective::VueVBindShorthandDirective and the
AnyVueDirective::VueDirective branch must strip a leading ':' from the argument
token before comparing to name_to_lookup and make the v-bind name check
ASCII-case-insensitive; update the arg name extraction (used in the chains that
call name_token().ok() and text_trimmed()) to obtain the token text, remove a
leading ':' (e.g. trim_start_matches(':')) and then compare with
eq_ignore_ascii_case(name_to_lookup), and change the v-bind detection (currently
t.text_trimmed() == "v-bind") to use eq_ignore_ascii_case so both branches
correctly recognise :alt, v-bind:alt and V-BIND:alt forms.
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
You are welcome! CodeRabbit had a good catch for the - let is_bind = d.name_token().is_ok_and(|t| t.text_trimmed() == "v-bind");
+ let is_bind = d.name_token().is_ok_and(|t| t.text_trimmed().eq_ignore_ascii_case("v-bind")); |
SchahinRohani
force-pushed
the
fix/use-alt-text-vue-dynamic-binding
branch
from
f4b63e5 to
7d5e401
Compare
SchahinRohani
force-pushed
the
fix/use-alt-text-vue-dynamic-binding
branch
from
7d5e401 to
8cefcc2
Compare
2 participants
Summary
Fixes #9349
useAltTextwas triggering a false positive in Vue SFCs when thealtattribute is bound dynamically via:alt(Vue v-bind shorthand) orv-bind:alt(explicit form).The root cause:
:altandv-bind:altare not parsed asHtmlAttributenodes but asAnyVueDirectivevariants (VueVBindShorthandDirectiveandVueDirectiverespectively). The previous implementation only calledfind_attribute_by_name("alt")which internally skips any non-HtmlAttributenodes, so Vue directives were never matched.The fix iterates over the full
HtmlAttributeListand explicitly handles:alt="..."— standardHtmlAttribute:alt="..."—VueVBindShorthandDirective, arg text is":alt"(leading colon stripped)v-bind:alt="..."—VueDirectivewithname_token == "v-bind", arg text is":alt"(leading colon stripped)Test Plan
Tested with:
Added
crates/biome_html_analyze/tests/specs/a11y/useAltText/vue/valid.vuecovering all three cases.All 132 tests pass:
Docs
No documentation changes needed — this is a bug fix for an existing rule, not a new rule or option.
AI Assistance Disclosure
This PR was developed with assistance from Claude (claude.ai). Claude helped debug the Biome AST structure (
AnyVueDirective,VueVBindShorthandDirective, etc.) and iterate on the implementation. The final solution was reviewed and understood by me throughout the process.