From 7e29f06930e977d5f2165260ca87c21f414cf62e Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 1 Oct 2024 10:26:04 +0100 Subject: [PATCH] fix(lint): options for `noLabelWithoutControl` are optional (#4138) --- CHANGELOG.md | 3 + crates/biome_analyze/CONTRIBUTING.md | 8 +- .../src/lint/a11y/no_label_without_control.rs | 205 +++++++++--------- .../@biomejs/backend-jsonrpc/src/workspace.ts | 6 +- .../@biomejs/biome/configuration_schema.json | 4 +- 5 files changed, 122 insertions(+), 104 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de1864582c7b..98f9c745add7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,9 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b ### Configuration +#### Bug fixes +- Fix [#4125](https://github.com/biomejs/biome/issues/4125), where `noLabelWithoutControl` options where incorrectly marked as mandatory. Contributed by @ematipico + ### Editors - Fix a case where CSS files weren't correctly linted using the default configuration. Contributed by @ematipico diff --git a/crates/biome_analyze/CONTRIBUTING.md b/crates/biome_analyze/CONTRIBUTING.md index 7caa1af4ac30..4635735b98c2 100644 --- a/crates/biome_analyze/CONTRIBUTING.md +++ b/crates/biome_analyze/CONTRIBUTING.md @@ -207,12 +207,18 @@ let options = ctx.options(); The compiler should warn you that `MyRuleOptions` does not implement some required types. We currently require implementing _serde_'s traits `Deserialize`/`Serialize`. + +Also, we use other `serde` macros to adjust the JSON configuration: +- `rename_all = "camelCase"`: it renames all fields in camel-case, so they are in line with the naming style of the `biome.json`. +- `deny_unknown_fields`: it raises an error if the configuration contains extraneous fields. +- `default`: it uses the `Default` value when the field is missing from `biome.json`. This macro makes the field optional. + You can simply use a derive macros: ```rust #[derive(Debug, Default, Clone, Serialize, Deserialize)] #[cfg_attr(feature = "schemars", derive(JsonSchema))] -#[serde(rename_all = "camelCase", deny_unknown_fields)] +#[serde(rename_all = "camelCase", deny_unknown_fields, default)] pub struct MyRuleOptions { #[serde(default, skip_serializing_if = "is_default")] main_behavior: Behavior, diff --git a/crates/biome_js_analyze/src/lint/a11y/no_label_without_control.rs b/crates/biome_js_analyze/src/lint/a11y/no_label_without_control.rs index 8ba4b85881e9..39fe4c431c8d 100644 --- a/crates/biome_js_analyze/src/lint/a11y/no_label_without_control.rs +++ b/crates/biome_js_analyze/src/lint/a11y/no_label_without_control.rs @@ -102,24 +102,17 @@ impl Rule for NoLabelWithoutControl { fn run(ctx: &RuleContext) -> Self::Signals { let node = ctx.query(); let options = ctx.options(); - let label_attributes = &options.label_attributes; - let label_components = &options.label_components; - let input_components = &options.input_components; - let element_name = node.name()?.name_value_token()?; let element_name = element_name.text_trimmed(); - let is_allowed_element = label_components - .iter() - .any(|label_component_name| label_component_name == element_name) + let is_allowed_element = options.has_element_name(element_name) || DEFAULT_LABEL_COMPONENTS.contains(&element_name); if !is_allowed_element { return None; } - let has_text_content = has_accessible_label(node, label_attributes); - let has_control_association = - has_for_attribute(node) || has_nested_control(node, input_components); + let has_text_content = options.has_accessible_label(node); + let has_control_association = has_for_attribute(node) || options.has_nested_control(node); if has_text_content && has_control_association { return None; @@ -159,7 +152,7 @@ impl Rule for NoLabelWithoutControl { #[derive(Clone, Debug, Default, Deserialize, Deserializable, Eq, PartialEq, Serialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] -#[serde(rename_all = "camelCase", deny_unknown_fields)] +#[serde(rename_all = "camelCase", deny_unknown_fields, default)] pub struct NoLabelWithoutControlOptions { /// Array of component names that should be considered the same as an `input` element. pub input_components: Vec, @@ -169,6 +162,108 @@ pub struct NoLabelWithoutControlOptions { pub label_components: Vec, } +impl NoLabelWithoutControlOptions { + /// Returns `true` whether the passed `attribute` meets one of the following conditions: + /// - Has a label attribute that corresponds to the `label_attributes` parameter + /// - Has a label among `DEFAULT_LABEL_ATTRIBUTES` + fn has_label_attribute(&self, attribute: &JsxAttribute) -> bool { + let Ok(attribute_name) = attribute.name().and_then(|name| name.name_token()) else { + return false; + }; + let attribute_name = attribute_name.text_trimmed(); + if !DEFAULT_LABEL_ATTRIBUTES.contains(&attribute_name) + && !self + .label_attributes + .iter() + .any(|name| name == attribute_name) + { + return false; + } + attribute + .initializer() + .and_then(|init| init.value().ok()) + .is_some_and(|v| has_label_attribute_value(&v)) + } + + /// Returns `true` whether the passed `jsx_tag` meets one of the following conditions: + /// - Has a label attribute that corresponds to the `label_attributes` parameter + /// - Has a label among `DEFAULT_LABEL_ATTRIBUTES` + /// - Has a child that acts as a label + fn has_accessible_label(&self, jsx_tag: &AnyJsxTag) -> bool { + let mut child_iter = jsx_tag.syntax().preorder(); + while let Some(event) = child_iter.next() { + match event { + WalkEvent::Enter(child) => match child.kind() { + JsSyntaxKind::JSX_EXPRESSION_CHILD + | JsSyntaxKind::JSX_SPREAD_CHILD + | JsSyntaxKind::JSX_TEXT => { + return true; + } + JsSyntaxKind::JSX_ELEMENT + | JsSyntaxKind::JSX_OPENING_ELEMENT + | JsSyntaxKind::JSX_CHILD_LIST + | JsSyntaxKind::JSX_SELF_CLOSING_ELEMENT + | JsSyntaxKind::JSX_ATTRIBUTE_LIST => {} + JsSyntaxKind::JSX_ATTRIBUTE => { + let attribute = JsxAttribute::unwrap_cast(child); + if self.has_label_attribute(&attribute) { + return true; + } + child_iter.skip_subtree(); + } + _ => { + child_iter.skip_subtree(); + } + }, + WalkEvent::Leave(_) => {} + } + } + false + } + + /// Returns whether the passed `AnyJsxTag` have a child that is considered an input component + /// according to the passed `input_components` parameter + fn has_nested_control(&self, jsx_tag: &AnyJsxTag) -> bool { + let mut child_iter = jsx_tag.syntax().preorder(); + while let Some(event) = child_iter.next() { + match event { + WalkEvent::Enter(child) => match child.kind() { + JsSyntaxKind::JSX_ELEMENT + | JsSyntaxKind::JSX_OPENING_ELEMENT + | JsSyntaxKind::JSX_CHILD_LIST + | JsSyntaxKind::JSX_SELF_CLOSING_ELEMENT => {} + _ => { + let Some(element_name) = AnyJsxElementName::cast(child) else { + child_iter.skip_subtree(); + continue; + }; + let Some(element_name) = element_name.name_value_token() else { + continue; + }; + let element_name = element_name.text_trimmed(); + if DEFAULT_INPUT_COMPONENTS.contains(&element_name) + || self + .input_components + .iter() + .any(|name| name == element_name) + { + return true; + } + } + }, + WalkEvent::Leave(_) => {} + } + } + false + } + + fn has_element_name(&self, element_name: &str) -> bool { + self.label_components + .iter() + .any(|label_component_name| label_component_name == element_name) + } +} + pub struct NoLabelWithoutControlState { pub has_text_content: bool, pub has_control_association: bool, @@ -201,94 +296,6 @@ fn has_for_attribute(jsx_tag: &AnyJsxTag) -> bool { }) } -/// Returns whether the passed `AnyJsxTag` have a child that is considered an input component -/// according to the passed `input_components` parameter -fn has_nested_control(jsx_tag: &AnyJsxTag, input_components: &[String]) -> bool { - let mut child_iter = jsx_tag.syntax().preorder(); - while let Some(event) = child_iter.next() { - match event { - WalkEvent::Enter(child) => match child.kind() { - JsSyntaxKind::JSX_ELEMENT - | JsSyntaxKind::JSX_OPENING_ELEMENT - | JsSyntaxKind::JSX_CHILD_LIST - | JsSyntaxKind::JSX_SELF_CLOSING_ELEMENT => {} - _ => { - let Some(element_name) = AnyJsxElementName::cast(child) else { - child_iter.skip_subtree(); - continue; - }; - let Some(element_name) = element_name.name_value_token() else { - continue; - }; - let element_name = element_name.text_trimmed(); - if DEFAULT_INPUT_COMPONENTS.contains(&element_name) - || input_components.iter().any(|name| name == element_name) - { - return true; - } - } - }, - WalkEvent::Leave(_) => {} - } - } - false -} - -/// Returns `true` whether the passed `jsx_tag` meets one of the following conditions: -/// - Has a label attribute that corresponds to the `label_attributes` parameter -/// - Has a label among `DEFAULT_LABEL_ATTRIBUTES` -/// - Has a child that acts as a label -fn has_accessible_label(jsx_tag: &AnyJsxTag, label_attributes: &[String]) -> bool { - let mut child_iter = jsx_tag.syntax().preorder(); - while let Some(event) = child_iter.next() { - match event { - WalkEvent::Enter(child) => match child.kind() { - JsSyntaxKind::JSX_EXPRESSION_CHILD - | JsSyntaxKind::JSX_SPREAD_CHILD - | JsSyntaxKind::JSX_TEXT => { - return true; - } - JsSyntaxKind::JSX_ELEMENT - | JsSyntaxKind::JSX_OPENING_ELEMENT - | JsSyntaxKind::JSX_CHILD_LIST - | JsSyntaxKind::JSX_SELF_CLOSING_ELEMENT - | JsSyntaxKind::JSX_ATTRIBUTE_LIST => {} - JsSyntaxKind::JSX_ATTRIBUTE => { - let attribute = JsxAttribute::unwrap_cast(child); - if has_label_attribute(&attribute, label_attributes) { - return true; - } - child_iter.skip_subtree(); - } - _ => { - child_iter.skip_subtree(); - } - }, - WalkEvent::Leave(_) => {} - } - } - false -} - -/// Returns `true` whether the passed `attribute` meets one of the following conditions: -/// - Has a label attribute that corresponds to the `label_attributes` parameter -/// - Has a label among `DEFAULT_LABEL_ATTRIBUTES` -fn has_label_attribute(attribute: &JsxAttribute, label_attributes: &[String]) -> bool { - let Ok(attribute_name) = attribute.name().and_then(|name| name.name_token()) else { - return false; - }; - let attribute_name = attribute_name.text_trimmed(); - if !DEFAULT_LABEL_ATTRIBUTES.contains(&attribute_name) - && !label_attributes.iter().any(|name| name == attribute_name) - { - return false; - } - attribute - .initializer() - .and_then(|init| init.value().ok()) - .is_some_and(|v| has_label_attribute_value(&v)) -} - /// Returns whether the passed `jsx_attribute_value` has a valid value inside it fn has_label_attribute_value(jsx_attribute_value: &AnyJsxAttributeValue) -> bool { match jsx_attribute_value { diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index 20ac81aa68a8..1adc459d76b6 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -2295,15 +2295,15 @@ export interface NoLabelWithoutControlOptions { /** * Array of component names that should be considered the same as an `input` element. */ - inputComponents: string[]; + inputComponents?: string[]; /** * Array of attributes that should be treated as the `label` accessible text content. */ - labelAttributes: string[]; + labelAttributes?: string[]; /** * Array of component names that should be considered the same as a `label` element. */ - labelComponents: string[]; + labelComponents?: string[]; } export interface ValidAriaRoleOptions { allowInvalidRoles: string[]; diff --git a/packages/@biomejs/biome/configuration_schema.json b/packages/@biomejs/biome/configuration_schema.json index e028d4f9e51a..522ac842bf49 100644 --- a/packages/@biomejs/biome/configuration_schema.json +++ b/packages/@biomejs/biome/configuration_schema.json @@ -2010,20 +2010,22 @@ }, "NoLabelWithoutControlOptions": { "type": "object", - "required": ["inputComponents", "labelAttributes", "labelComponents"], "properties": { "inputComponents": { "description": "Array of component names that should be considered the same as an `input` element.", + "default": [], "type": "array", "items": { "type": "string" } }, "labelAttributes": { "description": "Array of attributes that should be treated as the `label` accessible text content.", + "default": [], "type": "array", "items": { "type": "string" } }, "labelComponents": { "description": "Array of component names that should be considered the same as a `label` element.", + "default": [], "type": "array", "items": { "type": "string" } }