fix(useButtonType): better default behavior in Vue and Svelte#8995
fix(useButtonType): better default behavior in Vue and Svelte#8995
Conversation
|
github-actions
bot
added
A-Linter
Merging this PR will not alter performance
Comparing 
Footnotes
|
WalkthroughUpdated the use_button_type a11y lint to recognise framework-specific HTML sources (Svelte/Vue) and handle dynamic/shorthand attributes. is_button_element now accepts a RuleContext to decide case-sensitive vs case-insensitive tag matching. Added has_dynamic_attribute to detect shorthand/v-bind patterns. Attribute handling was adjusted to validate static string types against allowed values while treating dynamic expressions as non-blocking. AnyHtmlElement gained an attributes() accessor. New test fixtures for HTML, Svelte and Vue were added. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
- some files should be deleted
- the docs should be updated to reflect the new changes
- the code can be improved
dyc3
force-pushed
the
dyc3/use-button-type-fixes
branch
from
bfb15a5 to
8f7f3fe
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
🤖 Fix all issues with AI agents
In `@crates/biome_html_analyze/src/lint/a11y/use_button_type.rs`:
- Around line 145-187: has_dynamic_attribute currently only treats v-bind as
matching when it has a static argument; update the vue directive branch (the
checks around attr.as_any_vue_directive(), vue_directive.as_vue_directive(), and
vue_directive.as_vue_v_bind_shorthand_directive()) so that any v-bind without a
static argument is considered dynamic: if name_token.text_trimmed() == "v-bind"
and there is no arg (v-bind="attrs") return true, and if there is an arg but it
is not a static argument (v-bind:[foo]="bar" or shorthand :[foo]="bar") also
return true; keep the existing true-return for static-argument cases.
| /// Checks if a dynamic attribute (shorthand or directive) exists for the given name. | ||
| /// For example, `<button {type}>` (Svelte), `<button :type="foo">` (Vue), or `<button v-bind:type="foo">` (Vue). | ||
| fn has_dynamic_attribute(element: &AnyHtmlElement, name: &str) -> bool { | ||
| let Some(attributes) = element.attributes() else { | ||
| return false; | ||
| }; | ||
|
|
||
| attributes.iter().any(|attr| { | ||
| // Check if this is a HtmlSingleTextExpression (shorthand syntax) | ||
| if let Some(single_expr) = attr.as_html_single_text_expression() { | ||
| // Check if the expression text matches the attribute name we're looking for | ||
| if let Ok(expression) = single_expr.expression() | ||
| && let Ok(text) = expression.html_literal_token() | ||
| { | ||
| return text.text() == name; | ||
| } | ||
| } else if let Some(vue_directive) = attr.as_any_vue_directive() { | ||
| // Check for v-bind:type="foo" (longhand) | ||
| if let Some(dir) = vue_directive.as_vue_directive() | ||
| && let Ok(name_token) = dir.name_token() | ||
| && name_token.text_trimmed() == "v-bind" | ||
| && let Some(arg) = dir.arg() | ||
| && let Ok(arg) = arg.arg() | ||
| && let Some(static_arg) = arg.as_vue_static_argument() | ||
| && let Ok(name_token) = static_arg.name_token() | ||
| && name_token.text_trimmed() == name | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| // Check for :type="foo" (shorthand) | ||
| if let Some(dir) = vue_directive.as_vue_v_bind_shorthand_directive() | ||
| && let Ok(arg) = dir.arg() | ||
| && let Ok(arg) = arg.arg() | ||
| && let Some(static_arg) = arg.as_vue_static_argument() | ||
| && let Ok(name_token) = static_arg.name_token() | ||
| && name_token.text_trimmed() == name | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
| false | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's see the actual Vue directive definitions
rg -n "as_vue_directive|as_vue_v_bind" -g '*.rs' crates/biome_html_syntax | head -20Repository: biomejs/biome
Length of output: 317
🏁 Script executed:
#!/bin/bash
# Check the HTML syntax definitions for Vue directives
fd -e rs crates/biome_html_syntax | xargs grep -l "VueDirective\|v_bind" | head -10Repository: biomejs/biome
Length of output: 458
🏁 Script executed:
#!/bin/bash
# Check if tests exist for this rule with v-bind cases
fd -e rs crates/biome_html_analyze | xargs grep -l "v-bind\|v_bind" 2>/dev/null | head -10Repository: biomejs/biome
Length of output: 462
🏁 Script executed:
#!/bin/bash
# Find VueDirective definition in HTML syntax
rg -n "pub struct VueDirective|impl VueDirective" -g '*.rs' crates/biome_html_syntax -A 20 | head -60Repository: biomejs/biome
Length of output: 5422
🏁 Script executed:
#!/bin/bash
# Find test files for the use_button_type rule
find crates/biome_html_analyze -name '*use_button_type*' -o -name '*test*' | grep -i buttonRepository: biomejs/biome
Length of output: 115
🏁 Script executed:
#!/bin/bash
# Look at the actual arg() method definition for Vue directives
rg -n "fn arg\(" -g '*.rs' crates/biome_html_syntax -A 5 | head -40Repository: biomejs/biome
Length of output: 2766
🏁 Script executed:
#!/bin/bash
# Find AnyVueDirectiveArgument definition
rg -n "pub enum AnyVueDirectiveArgument|AnyVueDirectiveArgument {" -g '*.rs' crates/biome_html_syntax -A 10Repository: biomejs/biome
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Find test files and look for v-bind test cases
find crates/biome_html_analyze -type f -name '*.rs' | xargs grep -l "use_button_type"Repository: biomejs/biome
Length of output: 115
🏁 Script executed:
#!/bin/bash
# Look at the test snapshots for the rule
find crates/biome_html_analyze -type d -name '*use_button_type*'Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Find AnyVueDirectiveArgument definition - fixed regex
rg -n "enum AnyVueDirectiveArgument" -g '*.rs' crates/biome_html_syntax -A 15Repository: biomejs/biome
Length of output: 1510
🏁 Script executed:
#!/bin/bash
# Look at the actual test file to understand test structure
wc -l crates/biome_html_analyze/src/lint/a11y/use_button_type.rsRepository: biomejs/biome
Length of output: 119
🏁 Script executed:
#!/bin/bash
# Check if there's a test module in the file
rg -n "#\[test\]|mod test|#\[cfg\(test\)\]" crates/biome_html_analyze/src/lint/a11y/use_button_type.rsRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Read the full file to understand context
cat -n crates/biome_html_analyze/src/lint/a11y/use_button_type.rsRepository: biomejs/biome
Length of output: 8215
🏁 Script executed:
#!/bin/bash
# Find test files for this rule
find crates/biome_html_analyze -type f \( -name '*.json' -o -name '*.rs' \) | xargs grep -l "use_button_type" | head -10Repository: biomejs/biome
Length of output: 115
🏁 Script executed:
#!/bin/bash
# Find test directories/files for lint rules
find crates/biome_html_analyze -type f -name '*.rs' | xargs grep -l "a11y.*test\|test.*a11y" | head -5Repository: biomejs/biome
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Look for test snapshot files
find crates/biome_html_analyze -type d -name 'fixtures' | head -5Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search for test execution files
find crates -type f -name '*.rs' -path '*/test*' | xargs grep -l "use_button_type" 2>/dev/null | head -5Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Check the a11y module to understand test structure
rg -n "use_button_type\|#\[test\]" crates/biome_html_analyze/src/a11y.rs -A 3 | head -40Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Look for snapshots directory
find crates/biome_html_analyze -type d -name 'snapshots' -o -type d -name 'fixtures'Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search broader for test data
find crates/biome_html_analyze -type f -name '*button*' 2>/dev/nullRepository: biomejs/biome
Length of output: 115
🏁 Script executed:
#!/bin/bash
# Look for the test runner or macro implementation
rg -n "declare_lint_rule" crates/biome_html_analyze/src/lint/a11y/ -A 50 | head -100Repository: biomejs/biome
Length of output: 8863
🏁 Script executed:
#!/bin/bash
# Check how tests are executed - look in workspace or test runner
rg -n "use_button_type" crates/ --type rs 2>/dev/null | grep -v "\.rs:1:" | head -20Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Look at the actual Vue directive arg return type and behavior
rg -n "v-bind" -g '*.rs' crates/biome_html_syntax | head -20Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Let's verify by examining how Vue directives are parsed and structured
rg -n "VueDirectiveArgument\|VueDynamicArgument\|VueStaticArgument" -g '*.rs' crates/biome_html_syntax | head -30Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Check if there are any example test cases with v-bind in the codebase
rg -n "v-bind.*=" -g '*.html' -g '*.vue' 2>/dev/null | head -20Repository: biomejs/biome
Length of output: 2420
🏁 Script executed:
#!/bin/bash
# Look for test expectations or examples with Vue directives
rg -n "v-bind\|:type\|:.*=" -g '*.rs' crates/biome_html_analyze/src/lint/a11y/ | head -20Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Let's examine the parsed structure of v-bind examples
cat crates/biome_html_parser/tests/html_specs/ok/vue/v-bind-mixed.vueRepository: biomejs/biome
Length of output: 632
🏁 Script executed:
#!/bin/bash
# Check if v-bind without argument exists in tests
grep -r "v-bind=" crates/biome_html_parser/tests/ --include="*.vue" --include="*.html" 2>/dev/null | head -10Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Check the AnyVueDirectiveArgument usage in the codebase
rg -n "as_vue_dynamic_argument\|as_vue_static_argument" -g '*.rs' crates/biome_html_analyze | head -20Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Look at how the code currently handles Vue directives more carefully
cat -n crates/biome_html_analyze/src/lint/a11y/use_button_type.rs | sed -n '161,174p'Repository: biomejs/biome
Length of output: 794
🏁 Script executed:
#!/bin/bash
# Check if there's documentation about VueDirectiveArgument
rg -n "pub struct VueDirectiveArgument\|pub enum AnyVueDirectiveArgument" -g '*.rs' crates/biome_html_syntax -A 5Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Look for how VueDynamicArgument is defined
rg -n "pub struct VueDynamicArgument" -g '*.rs' crates/biome_html_syntax -A 8Repository: biomejs/biome
Length of output: 1649
🏁 Script executed:
#!/bin/bash
# Verify the logic: check if v-bind without arg returns None or something else
rg -n "VueDirective {" -g '*.rs' crates/biome_html_syntax -A 30 | head -50Repository: biomejs/biome
Length of output: 148
🏁 Script executed:
#!/bin/bash
# Check the test file that has v-bind without argument
grep -A 2 -B 2 "v-bind=" crates/biome_html_parser/tests/html_specs/ok/vue/v-bind-mixed.vueRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Verify arg() returns Option not Result
rg -n "pub fn arg\(&self\)" -g '*.rs' crates/biome_html_syntax -B 2 -A 3 | grep -A 3 "VueDirective"Repository: biomejs/biome
Length of output: 1925
🏁 Script executed:
#!/bin/bash
# Let's trace through exactly what happens with v-bind without argument
# Check if there's any handling of v-bind without arg in real code
rg -n "v-bind=\"" crates/ --type rs --type vue --type html 2>/dev/null | head -10Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Verify the logic flow: when arg() is None, the code returns false
# This confirms the review comment's concern
cat -n crates/biome_html_analyze/src/lint/a11y/use_button_type.rs | sed -n '145,187p' | grep -A 40 "v-bind"Repository: biomejs/biome
Length of output: 2226
🏁 Script executed:
#!/bin/bash
# Let's check if there are any discussions or related issues about this
rg -n "v-bind.*object\|object.*spread\|attrs" crates/biome_html_analyze/src/lint/a11y/use_button_type.rsRepository: biomejs/biome
Length of output: 39
Handle v-bind without a static argument as dynamic.
The current checks only return true for v-bind:type="foo" with a static argument. Cases like v-bind="attrs" (no argument) and v-bind:[foo]="bar" (dynamic argument) fail all the conditions and return false, creating false positives in wrapper patterns. Any v-bind directive lacking a static argument should assume the attribute is present.
Proposed fix
- if let Some(dir) = vue_directive.as_vue_directive()
- && let Ok(name_token) = dir.name_token()
- && name_token.text_trimmed() == "v-bind"
- && let Some(arg) = dir.arg()
- && let Ok(arg) = arg.arg()
- && let Some(static_arg) = arg.as_vue_static_argument()
- && let Ok(name_token) = static_arg.name_token()
- && name_token.text_trimmed() == name
- {
- return true;
- }
+ if let Some(dir) = vue_directive.as_vue_directive()
+ && let Ok(name_token) = dir.name_token()
+ && name_token.text_trimmed() == "v-bind"
+ {
+ // v-bind without arg (e.g. v-bind="attrs") is dynamic → assume present
+ let Some(arg) = dir.arg() else {
+ return true;
+ };
+ let Ok(arg) = arg.arg() else {
+ return true;
+ };
+ let Some(static_arg) = arg.as_vue_static_argument() else {
+ return true;
+ };
+ if let Ok(name_token) = static_arg.name_token()
+ && name_token.text_trimmed() == name
+ {
+ return true;
+ }
+ }🤖 Prompt for AI Agents
In `@crates/biome_html_analyze/src/lint/a11y/use_button_type.rs` around lines 145
- 187, has_dynamic_attribute currently only treats v-bind as matching when it
has a static argument; update the vue directive branch (the checks around
attr.as_any_vue_directive(), vue_directive.as_vue_directive(), and
vue_directive.as_vue_v_bind_shorthand_directive()) so that any v-bind without a
static argument is considered dynamic: if name_token.text_trimmed() == "v-bind"
and there is no arg (v-bind="attrs") return true, and if there is an arg but it
is not a static argument (v-bind:[foo]="bar" or shorthand :[foo]="bar") also
return true; keep the existing true-return for static-argument cases.
2 participants
Summary
This fixes 2 things about the HTML version of this rule when it's run on Vue or Svelte files:
typeis set dynamically and not with a static valueThe false negatives are OK because in practice these cases would occur in components that simply wrap
<button>for consistent styling across a codebase, as is common when using things like shadcn.I did the tests, but sonnet 4.5 did the implementation to make the tests pass.
No changeset because this rule hasn't been released yet.
Test Plan
snapshots
Docs