Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
chore: code suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Dec 8, 2022
1 parent 9e285f8 commit eae3d96
Show file tree
Hide file tree
Showing 9 changed files with 351 additions and 74 deletions.
2 changes: 1 addition & 1 deletion crates/rome_aria/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub mod roles;
pub use properties::AriaProperties;
pub(crate) use roles::AriaRoleDefinition;
pub use roles::AriaRoles;
use rome_aria_metadata::{AriaPropertiesEnum, AriaPropertyTypeEnum};
pub use rome_aria_metadata::{AriaPropertiesEnum, AriaPropertyTypeEnum};

/// It checks if an ARIA property is valid
///
Expand Down
8 changes: 4 additions & 4 deletions crates/rome_aria/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ macro_rules! define_role {
}

impl $crate::AriaRoleDefinition for $id {
fn properties<'a>(&self) -> std::slice::Iter<'a, (&str, bool)> {
fn properties(&self) -> std::slice::Iter<(&str, bool)> {
$id::PROPS.iter()
}

fn roles<'a>(&self) -> std::slice::Iter<'a, &str> {
fn roles(&self) -> std::slice::Iter<&str> {
$id::ROLES.iter()
}
}
Expand All @@ -40,11 +40,11 @@ macro_rules! define_property {
}

impl AriaPropertyDefinition for $id {
fn values<'a>(&self) -> std::slice::Iter<'a, &str> {
fn values(&self) -> std::slice::Iter<&'static str> {
$id::VALUES.iter()
}

fn property_type<'a>(&self) -> $crate::AriaPropertyTypeEnum {
fn property_type(&self) -> $crate::AriaPropertyTypeEnum {
// SAFETY: PROPERTY_TYPE is internal and should not contain extraneous properties
$crate::AriaPropertyTypeEnum::from_str($id::PROPERTY_TYPE).unwrap()
}
Expand Down
6 changes: 3 additions & 3 deletions crates/rome_aria/src/properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ define_property! {

define_property! {
AriaDropeffect {
PROPERTY_TYPE: "id",
PROPERTY_TYPE: "tokenlist",
VALUES: ["copy", "execute", "link", "move", "none", "popup"],
}
}
Expand Down Expand Up @@ -342,7 +342,7 @@ define_property! {
pub struct AriaProperties;

impl AriaProperties {
pub fn get_property(&self, property_name: &str) -> Option<&dyn AriaPropertyDefinition> {
pub fn get_property<'a>(&self, property_name: &str) -> Option<&'a dyn AriaPropertyDefinition> {
Some(match property_name {
"aria-activedescendant" => &AriaActivedescendant as &dyn AriaPropertyDefinition,
"aria-autocomplete" => &AriaAutocomplete as &dyn AriaPropertyDefinition,
Expand Down Expand Up @@ -398,7 +398,7 @@ impl AriaProperties {

pub trait AriaPropertyDefinition: Debug {
/// Returns the allowed values by this property
fn values<'a>(&self) -> Iter<'a, &str>;
fn values(&self) -> Iter<&str>;

/// Returns the property type
fn property_type(&self) -> AriaPropertyTypeEnum;
Expand Down
6 changes: 3 additions & 3 deletions crates/rome_aria/src/roles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ pub trait AriaRoleDefinition: Debug {
/// let properties = checkbox_role.properties();
/// assert_eq!(properties.len(), 2);
/// ```
fn properties<'a>(&self) -> Iter<'a, (&str, bool)>;
fn properties(&self) -> Iter<(&str, bool)>;

/// It returns an iterator over the possible roles of this definition
fn roles<'a>(&self) -> Iter<'a, &str>;
fn roles(&self) -> Iter<&str>;

/// Given a [aria property](ARIA_PROPERTIES) as input, it checks if it's required
/// for the current role.
Expand All @@ -45,7 +45,7 @@ pub trait AriaRoleDefinition: Debug {
if is_aria_property_valid(property_to_check) {
let property_to_check = AriaPropertiesEnum::from_str(property_to_check);
if let Ok(property_to_check) = property_to_check {
for (property, required) in self.properties().as_ref() {
for (property, required) in self.properties() {
let property = AriaPropertiesEnum::from_str(property).unwrap();
if property == property_to_check {
return *required;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use crate::aria_services::Aria;
use rome_analyze::context::RuleContext;
use rome_analyze::{declare_rule, Rule, RuleDiagnostic};
use rome_aria::AriaPropertyTypeEnum;
use rome_console::markup;
use rome_js_syntax::{
AnyJsExpression, AnyJsLiteralExpression, AnyJsxAttributeValue, JsSyntaxToken, JsxAttribute,
TextRange,
};
use rome_rowan::{AstNode, AstNodeList};
use std::slice::Iter;

declare_rule! {
/// Enforce that ARIA state and property values are valid.
Expand All @@ -24,6 +26,14 @@ declare_rule! {
/// <span aria-labelledby="" >some text</span>
/// ```
///
/// ```jsx, expect_diagnostic
/// <span aria-valuemax="hey">some text</span>
/// ```
///
/// ```jsx, expect_diagnostic
/// <span aria-orientation="hey">some text</span>
/// ```
///
/// ### Valid
///
/// ```jsx
Expand All @@ -48,8 +58,9 @@ declare_rule! {

pub(crate) struct UseAriaProptypesState {
attribute_value_range: TextRange,
allowed_values: Vec<String>,
allowed_values: Iter<'static, &'static str>,
attribute_name: JsSyntaxToken,
property_type: AriaPropertyTypeEnum,
}

impl Rule for UseAriaPropTypes {
Expand All @@ -76,15 +87,21 @@ impl Rule for UseAriaPropTypes {
// Early error, the template literal is empty
return Some(UseAriaProptypesState {
attribute_value_range,
allowed_values: aria_property
.values()
.map(|value| value.to_string())
.collect::<Vec<_>>(),
allowed_values: aria_property.values(),
attribute_name,
property_type: aria_property.property_type(),
});
} else {
None
}
template
.elements()
.iter()
.next()
.and_then(|chunk| {
chunk
.as_js_template_chunk_element()
.and_then(|t| t.template_chunk_token().ok())
})
.map(|t| t.token_text_trimmed())
}
AnyJsExpression::AnyJsLiteralExpression(
AnyJsLiteralExpression::JsStringLiteralExpression(string),
Expand All @@ -98,11 +115,9 @@ impl Rule for UseAriaPropTypes {
if !aria_property.contains_correct_value(attribute_text.text()) {
return Some(UseAriaProptypesState {
attribute_value_range,
allowed_values: aria_property
.values()
.map(|value| value.to_string())
.collect::<Vec<_>>(),
allowed_values: aria_property.values(),
attribute_name,
property_type: aria_property.property_type(),
});
}
}
Expand All @@ -112,19 +127,73 @@ impl Rule for UseAriaPropTypes {

fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let attribute_name = state.attribute_name.text_trimmed();
Some(
RuleDiagnostic::new(
rule_category!(),
state.attribute_value_range,
markup! {
let diagnostic = RuleDiagnostic::new(
rule_category!(),
state.attribute_value_range,
markup! {
"The value of the ARIA attribute "<Emphasis>{attribute_name}</Emphasis>" is not correct."
},
).footer_list(
markup!{
"The supported values for the "<Emphasis>{attribute_name}</Emphasis>" attribute are:"
);

let diagnostic = match state.property_type {
AriaPropertyTypeEnum::Boolean => {
diagnostic.footer_list(
markup!{
"The only supported values for the "<Emphasis>{attribute_name}</Emphasis>" is one of the following:"
},
&["true", "false"]
)
}
AriaPropertyTypeEnum::Integer => {
diagnostic.note(
markup!{
"The only value supported is a number without fractional components."
}
)
}
AriaPropertyTypeEnum::Id |
AriaPropertyTypeEnum::Idlist |
AriaPropertyTypeEnum::String => {
diagnostic.note(
markup!{
"The only supported value is text."
}
)
}

AriaPropertyTypeEnum::Number => {
diagnostic.note(
markup!{
"The only supported value is number."
}
)
}
AriaPropertyTypeEnum::Token => {
diagnostic.footer_list(
markup!{
"The only supported value for the "<Emphasis>{attribute_name}</Emphasis>" is one of the following:"
},
&state.allowed_values
)
)
state.allowed_values.as_slice()
)
}
AriaPropertyTypeEnum::Tokenlist => {
diagnostic.footer_list(
markup!{
"The values supported for "<Emphasis>{attribute_name}</Emphasis>" are one or more of the following:"
},
state.allowed_values.as_slice()
)
}
AriaPropertyTypeEnum::Tristate => {
diagnostic.footer_list(
markup!{
"The only supported value for the "<Emphasis>{attribute_name}</Emphasis>" one of the following:"
},
&["true", "false", "mixed"]
)
}
};

Some(diagnostic)
}
}
8 changes: 2 additions & 6 deletions crates/rome_js_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,17 +226,13 @@ mod tests {
String::from_utf8(buffer).unwrap()
}

const SOURCE: &str = r#"something.forEach((Element, index) => {
return <List
><div key={index}>foo</div>
</List>;
})"#;
const SOURCE: &str = r#"<span aria-labelledby={``} ></span>"#;

let parsed = parse(SOURCE, FileId::zero(), SourceType::jsx());

let mut error_ranges: Vec<TextRange> = Vec::new();
let options = AnalyzerOptions::default();
let rule_filter = RuleFilter::Rule("suspicious", "noArrayIndexKey");
let rule_filter = RuleFilter::Rule("nursery", "useAriaPropTypes");
analyze(
FileId::zero(),
&parsed.tree(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
var a = <span role="checkbox" aria-checked="test" ></span>;
var a = <span aria-autocomplete="test" ></span>;
var a = <span aria-invalid="foo"></span>;
var a = <span aria-invalid={`foo`} ></span>;
var a = <span aria-invalid={"foo"} ></span>;
var a = <span aria-errormessage="" ></span>;
var a = <span aria-relevant="fancy" ></span>;
var a = <span aria-labelledby="" ></span>;
var a = <span aria-labelledby={``} ></span>;
var a = <span aria-labelledby={""} ></span>;
var a = <span aria-details=""></span>;
var a = <span aria-setsize="hey"></span>;
var a = <span aria-valuemax="hey"></span>;
var a = <span aria-dropeffect="hey"></span>;
var a = <span aria-orientation="hey"></span>;
Loading

0 comments on commit eae3d96

Please sign in to comment.