Skip to content

Commit

Permalink
fix(lint): options for noLabelWithoutControl are optional (#4138)
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico authored Oct 1, 2024
1 parent 10a1371 commit 7e29f06
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 104 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion crates/biome_analyze/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
205 changes: 106 additions & 99 deletions crates/biome_js_analyze/src/lint/a11y/no_label_without_control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,24 +102,17 @@ impl Rule for NoLabelWithoutControl {
fn run(ctx: &RuleContext<Self>) -> 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;
Expand Down Expand Up @@ -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<String>,
Expand All @@ -169,6 +162,108 @@ pub struct NoLabelWithoutControlOptions {
pub label_components: Vec<String>,
}

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,
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions packages/@biomejs/backend-jsonrpc/src/workspace.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion packages/@biomejs/biome/configuration_schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 7e29f06

Please sign in to comment.