chore: abstract html tag matcher#9988
Conversation
🦋 Changeset detectedLatest commit: 6962357 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 |
Merging this PR will not alter performance
Comparing Footnotes
|
372c009 to
d9e15d1
Compare
d9e15d1 to
76b0c80
Compare
|
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 selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis change centralises source-aware HTML tag matching and narrows element types across the analyzer. It adds 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)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/biome_html_analyze/src/lint/nursery/use_scoped_styles.rs (1)
100-109: Consider reusing the cachedsource_type.You've already derived
source_typeat line 83. Lines 100 and 109 callctx.source_type::<HtmlFileSource>()again—consider using the local variable instead.♻️ Proposed fix
- if ctx.source_type::<HtmlFileSource>().is_vue() { + if source_type.is_vue() { let has_scoped = attributes.find_by_name("scoped").is_some(); let has_module = attributes.find_by_name("module").is_some(); if has_scoped || has_module { return None; } else { return Some(GlobalStylesKind::Vue); } - } else if ctx.source_type::<HtmlFileSource>().is_astro() { + } else if source_type.is_astro() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_html_analyze/src/lint/nursery/use_scoped_styles.rs` around lines 100 - 109, The code calls ctx.source_type::<HtmlFileSource>() multiple times inside use_scoped_styles.rs (e.g., in the branches that check .is_vue() and .is_astro()); reuse the previously computed local variable (the cached source_type you created earlier) instead of calling ctx.source_type::<HtmlFileSource>() again — replace subsequent ctx.source_type::<HtmlFileSource>().is_vue()/is_astro() checks with source_type.is_vue()/source_type.is_astro() to avoid redundant lookups while preserving the existing conditional logic that returns Some(GlobalStylesKind::Vue) when no scoped/module attributes are present.crates/biome_html_analyze/src/lint/nursery/no_script_url.rs (1)
62-73: Consider queryingAnyHtmlTagElementdirectly.The rule queries
HtmlOpeningElementbut then converts it toAnyHtmlTagElementfor the tag check. This works correctly, though for consistency with other refactored rules in this PR, you could queryAst<AnyHtmlTagElement>directly and avoid the conversion on line 71.That said, the current approach is functionally correct — self-closing
<a/>tags are unusual in practice.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_html_analyze/src/lint/nursery/no_script_url.rs` around lines 62 - 73, Change the rule to query AnyHtmlTagElement directly instead of HtmlOpeningElement: update the type alias Query from Ast<HtmlOpeningElement> to Ast<AnyHtmlTagElement>, and in run(ctx: &RuleContext<Self>) use ctx.query() as an AnyHtmlTagElement (removing the conversion AnyHtmlTagElement::from(element.clone())). Keep the same source_type check and is_html_tag call but pass the queried AnyHtmlTagElement directly to is_html_tag; adjust any variable names/types in run accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_html_analyze/src/lint/nursery/no_script_url.rs`:
- Around line 62-73: Change the rule to query AnyHtmlTagElement directly instead
of HtmlOpeningElement: update the type alias Query from Ast<HtmlOpeningElement>
to Ast<AnyHtmlTagElement>, and in run(ctx: &RuleContext<Self>) use ctx.query()
as an AnyHtmlTagElement (removing the conversion
AnyHtmlTagElement::from(element.clone())). Keep the same source_type check and
is_html_tag call but pass the queried AnyHtmlTagElement directly to is_html_tag;
adjust any variable names/types in run accordingly.
In `@crates/biome_html_analyze/src/lint/nursery/use_scoped_styles.rs`:
- Around line 100-109: The code calls ctx.source_type::<HtmlFileSource>()
multiple times inside use_scoped_styles.rs (e.g., in the branches that check
.is_vue() and .is_astro()); reuse the previously computed local variable (the
cached source_type you created earlier) instead of calling
ctx.source_type::<HtmlFileSource>() again — replace subsequent
ctx.source_type::<HtmlFileSource>().is_vue()/is_astro() checks with
source_type.is_vue()/source_type.is_astro() to avoid redundant lookups while
preserving the existing conditional logic that returns
Some(GlobalStylesKind::Vue) when no scoped/module attributes are present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a08b06ee-e62d-42f1-887b-fe5145208da6
⛔ Files ignored due to path filters (16)
crates/biome_cli/tests/snapshots/main_cases_handle_astro_files/full_support.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_svelte_files/full_support.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_svelte_files/full_support_ts.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_vue_files/full_support.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_handle_vue_files/full_support_ts.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/useAltText/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/useButtonType/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/useButtonType/invalid.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/useButtonType/invalid.vue.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/useHtmlLang/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/useIframeTitle/astro/invalid.astro.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/useIframeTitle/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/useIframeTitle/svelte/invalid.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/useIframeTitle/vue/invalid.vue.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/useValidAriaRole/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/nursery/useIframeSandbox/invalid.html.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (29)
crates/biome_html_analyze/src/a11y.rscrates/biome_html_analyze/src/a11y_tests.rscrates/biome_html_analyze/src/lib.rscrates/biome_html_analyze/src/lint/a11y/no_autofocus.rscrates/biome_html_analyze/src/lint/a11y/no_distracting_elements.rscrates/biome_html_analyze/src/lint/a11y/no_header_scope.rscrates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rscrates/biome_html_analyze/src/lint/a11y/no_redundant_alt.rscrates/biome_html_analyze/src/lint/a11y/no_svg_without_title.rscrates/biome_html_analyze/src/lint/a11y/use_alt_text.rscrates/biome_html_analyze/src/lint/a11y/use_anchor_content.rscrates/biome_html_analyze/src/lint/a11y/use_aria_props_for_role.rscrates/biome_html_analyze/src/lint/a11y/use_button_type.rscrates/biome_html_analyze/src/lint/a11y/use_html_lang.rscrates/biome_html_analyze/src/lint/a11y/use_iframe_title.rscrates/biome_html_analyze/src/lint/a11y/use_media_caption.rscrates/biome_html_analyze/src/lint/a11y/use_valid_aria_role.rscrates/biome_html_analyze/src/lint/a11y/use_valid_lang.rscrates/biome_html_analyze/src/lint/nursery/no_ambiguous_anchor_text.rscrates/biome_html_analyze/src/lint/nursery/no_inline_styles.rscrates/biome_html_analyze/src/lint/nursery/no_script_url.rscrates/biome_html_analyze/src/lint/nursery/no_sync_scripts.rscrates/biome_html_analyze/src/lint/nursery/use_iframe_sandbox.rscrates/biome_html_analyze/src/lint/nursery/use_scoped_styles.rscrates/biome_html_analyze/src/lint/nursery/use_vue_hyphenated_attributes.rscrates/biome_html_analyze/src/lint/nursery/use_vue_valid_template_root.rscrates/biome_html_analyze/src/lint/nursery/use_vue_vapor.rscrates/biome_html_analyze/src/utils.rscrates/biome_html_syntax/src/element_ext.rs
ematipico
left a comment
There was a problem hiding this comment.
This change affected diagnostics, which are emitted to users. This means that it needs a changeset.
Summary
Test Plan
Docs