Skip to content

Commit

Permalink
feat(lint): add useGoogleFontDisplay rule (#4264)
Browse files Browse the repository at this point in the history
Co-authored-by: togami <62130798+togami2864@users.noreply.github.com>
  • Loading branch information
kaioduarte and togami2864 authored Oct 17, 2024
1 parent 7fffb27 commit 2a05cd4
Show file tree
Hide file tree
Showing 13 changed files with 476 additions and 14 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
- Add [noImgElement](https://biomejs.dev/linter/rules/no-img-element/). Contributed by @kaioduarte
- Add [guardForIn](https://biomejs.dev/linter/rules/guard-for-in/). Contributed by @fireairforce
- Add [noUselessStringRaw](https://github.com/biomejs/biome/pull/4263). Contributed by @fireairforce
- Add [useGoogleFontDisplay](https://biomejs.dev/linter/rules/use-google-font-display/). Contributed by @kaioduarte

#### Bug Fixes

Expand Down Expand Up @@ -186,7 +187,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
#### New features

- Add support for parsing the defer attribute in import statements ([#4215](https://github.com/biomejs/biome/issues/4215)).

```js
import defer * as myModule from "my-module";
```
Expand Down
10 changes: 10 additions & 0 deletions crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs

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

46 changes: 33 additions & 13 deletions crates/biome_configuration/src/analyzer/linter/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3417,6 +3417,10 @@ pub struct Nursery {
#[doc = "Require explicit return types on functions and class methods."]
#[serde(skip_serializing_if = "Option::is_none")]
pub use_explicit_type: Option<RuleConfiguration<biome_js_analyze::options::UseExplicitType>>,
#[doc = "Enforces the use of a recommended display strategy with Google Fonts."]
#[serde(skip_serializing_if = "Option::is_none")]
pub use_google_font_display:
Option<RuleConfiguration<biome_js_analyze::options::UseGoogleFontDisplay>>,
#[doc = "Require for-in loops to include an if statement."]
#[serde(skip_serializing_if = "Option::is_none")]
pub use_guard_for_in: Option<RuleConfiguration<biome_js_analyze::options::UseGuardForIn>>,
Expand Down Expand Up @@ -3497,6 +3501,7 @@ impl Nursery {
"useConsistentMemberAccessibility",
"useDeprecatedReason",
"useExplicitType",
"useGoogleFontDisplay",
"useGuardForIn",
"useImportRestrictions",
"useSortedClasses",
Expand Down Expand Up @@ -3534,7 +3539,7 @@ impl Nursery {
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32]),
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[37]),
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[38]),
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[43]),
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[44]),
];
const ALL_RULES_AS_FILTERS: &'static [RuleFilter<'static>] = &[
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[0]),
Expand Down Expand Up @@ -3583,6 +3588,7 @@ impl Nursery {
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[43]),
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[44]),
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[45]),
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[46]),
];
#[doc = r" Retrieves the recommended rules"]
pub(crate) fn is_recommended_true(&self) -> bool {
Expand Down Expand Up @@ -3799,36 +3805,41 @@ impl Nursery {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[39]));
}
}
if let Some(rule) = self.use_guard_for_in.as_ref() {
if let Some(rule) = self.use_google_font_display.as_ref() {
if rule.is_enabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[40]));
}
}
if let Some(rule) = self.use_import_restrictions.as_ref() {
if let Some(rule) = self.use_guard_for_in.as_ref() {
if rule.is_enabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[41]));
}
}
if let Some(rule) = self.use_sorted_classes.as_ref() {
if let Some(rule) = self.use_import_restrictions.as_ref() {
if rule.is_enabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[42]));
}
}
if let Some(rule) = self.use_strict_mode.as_ref() {
if let Some(rule) = self.use_sorted_classes.as_ref() {
if rule.is_enabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[43]));
}
}
if let Some(rule) = self.use_trim_start_end.as_ref() {
if let Some(rule) = self.use_strict_mode.as_ref() {
if rule.is_enabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[44]));
}
}
if let Some(rule) = self.use_valid_autocomplete.as_ref() {
if let Some(rule) = self.use_trim_start_end.as_ref() {
if rule.is_enabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[45]));
}
}
if let Some(rule) = self.use_valid_autocomplete.as_ref() {
if rule.is_enabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[46]));
}
}
index_set
}
pub(crate) fn get_disabled_rules(&self) -> FxHashSet<RuleFilter<'static>> {
Expand Down Expand Up @@ -4033,36 +4044,41 @@ impl Nursery {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[39]));
}
}
if let Some(rule) = self.use_guard_for_in.as_ref() {
if let Some(rule) = self.use_google_font_display.as_ref() {
if rule.is_disabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[40]));
}
}
if let Some(rule) = self.use_import_restrictions.as_ref() {
if let Some(rule) = self.use_guard_for_in.as_ref() {
if rule.is_disabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[41]));
}
}
if let Some(rule) = self.use_sorted_classes.as_ref() {
if let Some(rule) = self.use_import_restrictions.as_ref() {
if rule.is_disabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[42]));
}
}
if let Some(rule) = self.use_strict_mode.as_ref() {
if let Some(rule) = self.use_sorted_classes.as_ref() {
if rule.is_disabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[43]));
}
}
if let Some(rule) = self.use_trim_start_end.as_ref() {
if let Some(rule) = self.use_strict_mode.as_ref() {
if rule.is_disabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[44]));
}
}
if let Some(rule) = self.use_valid_autocomplete.as_ref() {
if let Some(rule) = self.use_trim_start_end.as_ref() {
if rule.is_disabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[45]));
}
}
if let Some(rule) = self.use_valid_autocomplete.as_ref() {
if rule.is_disabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[46]));
}
}
index_set
}
#[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"]
Expand Down Expand Up @@ -4259,6 +4275,10 @@ impl Nursery {
.use_explicit_type
.as_ref()
.map(|conf| (conf.level(), conf.get_options())),
"useGoogleFontDisplay" => self
.use_google_font_display
.as_ref()
.map(|conf| (conf.level(), conf.get_options())),
"useGuardForIn" => self
.use_guard_for_in
.as_ref()
Expand Down
2 changes: 2 additions & 0 deletions crates/biome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,9 @@ define_categories! {
"lint/nursery/useConsistentCurlyBraces": "https://biomejs.dev/linter/rules/use-consistent-curly-braces",
"lint/nursery/useConsistentMemberAccessibility": "https://biomejs.dev/linter/rules/use-consistent-member-accessibility",
"lint/nursery/useDeprecatedReason": "https://biomejs.dev/linter/rules/use-deprecated-reason",
"lint/nursery/useExplicitFunctionReturnType": "https://biomejs.dev/linter/rules/use-explicit-function-return-type",
"lint/nursery/useExplicitType": "https://biomejs.dev/linter/rules/use-explicit-function-return-type",
"lint/nursery/useGoogleFontDisplay": "https://biomejs.dev/linter/rules/use-google-font-display",
"lint/nursery/useGuardForIn": "https://biomejs.dev/linter/rules/use-guard-for-in",
"lint/nursery/useImportRestrictions": "https://biomejs.dev/linter/rules/use-import-restrictions",
"lint/nursery/useJsxCurlyBraceConvention": "https://biomejs.dev/linter/rules/use-jsx-curly-brace-convention",
Expand Down
2 changes: 2 additions & 0 deletions crates/biome_js_analyze/src/lint/nursery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub mod use_component_export_only_modules;
pub mod use_consistent_curly_braces;
pub mod use_consistent_member_accessibility;
pub mod use_explicit_type;
pub mod use_google_font_display;
pub mod use_guard_for_in;
pub mod use_import_restrictions;
pub mod use_sorted_classes;
Expand Down Expand Up @@ -73,6 +74,7 @@ declare_lint_group! {
self :: use_consistent_curly_braces :: UseConsistentCurlyBraces ,
self :: use_consistent_member_accessibility :: UseConsistentMemberAccessibility ,
self :: use_explicit_type :: UseExplicitType ,
self :: use_google_font_display :: UseGoogleFontDisplay ,
self :: use_guard_for_in :: UseGuardForIn ,
self :: use_import_restrictions :: UseImportRestrictions ,
self :: use_sorted_classes :: UseSortedClasses ,
Expand Down
125 changes: 125 additions & 0 deletions crates/biome_js_analyze/src/lint/nursery/use_google_font_display.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
use biome_analyze::{
context::RuleContext, declare_lint_rule, Ast, Rule, RuleDiagnostic, RuleSource, RuleSourceKind,
};
use biome_console::markup;
use biome_js_syntax::jsx_ext::AnyJsxElement;
use biome_rowan::TextRange;

declare_lint_rule! {
/// Enforces the use of a recommended `display` strategy with Google Fonts.
///
/// The `display` property controls how a font is displayed while it is loading. When using Google Fonts,
/// it's important to specify an appropriate value for this property to ensure good user experience and prevent layout shifts.
///
/// This rule flags the absence of the `display` parameter, or the usage of less optimal values such as `auto`, `block`, or `fallback`.
/// Using `&display=optional` is generally recommended as it minimizes the risk of invisible text or layout shifts.
/// In cases where swapping to the custom font after it has loaded is important, consider using `&display=swap`.
///
/// ## Examples
///
/// ### Invalid
///
/// ```jsx,expect_diagnostic
/// <link href="https://fonts.googleapis.com/css2?family=Krona+One" />
/// ```
///
/// ```jsx,expect_diagnostic
/// <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=auto" />
/// ```
///
/// ```jsx,expect_diagnostic
/// <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=block" />
/// ```
///
/// ```jsx,expect_diagnostic
/// <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=fallback" />
/// ```
///
/// ### Valid
///
/// ```jsx
/// <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=optional" rel="stylesheet" />
/// ```
///
/// ```jsx
/// <link href="https://fonts.googleapis.com/css2?display=unknown" rel="stylesheet" />
/// ```
///
/// ```jsx
/// <link rel="stylesheet" />
/// ```
pub UseGoogleFontDisplay {
version: "next",
name: "useGoogleFontDisplay",
language: "jsx",
sources: &[RuleSource::EslintNext("google-font-display")],
source_kind: RuleSourceKind::SameLogic,
recommended: false,
}
}

const FORBIDDEN_VALUES: [&str; 3] = ["auto", "block", "fallback"];

pub enum FontDisplayIssue {
MissingDisplayParam,
ForbiddenValue,
}

impl Rule for UseGoogleFontDisplay {
type Query = Ast<AnyJsxElement>;
type State = (FontDisplayIssue, TextRange);
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let element = ctx.query();

if element.name().ok()?.name_value_token()?.text_trimmed() != "link" {
return None;
}

let href = element.find_attribute_by_name("href")?;
let initializer = href.initializer()?.value().ok()?.as_static_value()?;
let href_text = initializer.as_string_constant()?;

if !href_text.starts_with("https://fonts.googleapis.com/css") {
return None;
}

let display_param = href_text
.split('?')
.last()?
.split('&')
.find(|p| p.starts_with("display="));
let range = initializer.range();

if let Some(display_param) = display_param {
for forbidden_value in FORBIDDEN_VALUES {
if display_param.ends_with(forbidden_value) {
return Some((FontDisplayIssue::ForbiddenValue, range));
}
}
} else {
return Some((FontDisplayIssue::MissingDisplayParam, range));
}

None
}

fn diagnostic(_: &RuleContext<Self>, (issue, range): &Self::State) -> Option<RuleDiagnostic> {
let title = match issue {
FontDisplayIssue::MissingDisplayParam => markup! {
"The Google Font link is missing the "<Emphasis>"display"</Emphasis>" parameter."
},
FontDisplayIssue::ForbiddenValue => markup! {
"The Google Font link has a non-recommended "<Emphasis>"display"</Emphasis>" value."
},
};

Some(RuleDiagnostic::new(rule_category!(), range, title).note(
markup! {
"Use "<Emphasis>"&display=optional"</Emphasis>" to prevent invisible text and layout shifts. If font swapping is important, use "<Emphasis>"&display=swap"</Emphasis>"."
}
))
}
}
2 changes: 2 additions & 0 deletions crates/biome_js_analyze/src/options.rs

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<>
<link href="https://fonts.googleapis.com/css2?family=Krona+One" />
<link href="https://fonts.googleapis.com/css2?family=Krona+One&display=auto" />
<link href="https://fonts.googleapis.com/css2?family=Krona+One&display=block" />
<link href="https://fonts.googleapis.com/css2?family=Krona+One&display=fallback" />

<link href={"https://fonts.googleapis.com/css2?family=Krona+One"} />
<link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=auto"} />
<link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=block"} />
<link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=fallback"} />

<link href={`https://fonts.googleapis.com/css2?family=Krona+One`} />
<link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=auto`} />
<link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=block`} />
<link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=fallback`} />
</>
Loading

0 comments on commit 2a05cd4

Please sign in to comment.