Skip to content

Commit

Permalink
refactor(linter): remove usage of url crate
Browse files Browse the repository at this point in the history
  • Loading branch information
camchenry committed Feb 1, 2025
1 parent 580e5e6 commit 325753b
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 29 deletions.
2 changes: 0 additions & 2 deletions Cargo.lock

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

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ tower-lsp = "0.20.0"
tracing-subscriber = "0.3.19"
tsify = "0.4.5"
ureq = { version = "3.0.0", default-features = false }
url = "2.5.4"
walkdir = "2.5.0"
wasm-bindgen = "0.2.99"

Expand Down
1 change: 0 additions & 1 deletion crates/oxc_linter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ schemars = { workspace = true, features = ["indexmap2"] }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
simdutf8 = { workspace = true }
url = { workspace = true }

[dev-dependencies]
insta = { workspace = true }
Expand Down
16 changes: 4 additions & 12 deletions crates/oxc_linter/src/rules/nextjs/google_font_display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use oxc_span::{GetSpan, Span};
use crate::{
context::LintContext,
rule::Rule,
utils::{get_string_literal_prop_value, has_jsx_prop_ignore_case},
utils::{find_url_query_value, get_string_literal_prop_value, has_jsx_prop_ignore_case},
AstNode,
};

Expand Down Expand Up @@ -102,21 +102,13 @@ impl Rule for GoogleFontDisplay {
};

if href_prop_value.starts_with("https://fonts.googleapis.com/css") {
let Ok(url) = url::Url::parse(href_prop_value) else {
return;
};

let Some((_, display_value)) = url.query_pairs().find(|(key, _)| key == "display")
else {
let Some(display_value) = find_url_query_value(href_prop_value, "display") else {
ctx.diagnostic(font_display_parameter_missing(jsx_opening_element_name.span));
return;
};

if matches!(&*display_value, "auto" | "block" | "fallback") {
ctx.diagnostic(not_recommended_font_display_value(
href_prop.span(),
&display_value,
));
if matches!(display_value, "auto" | "block" | "fallback") {
ctx.diagnostic(not_recommended_font_display_value(href_prop.span(), display_value));
}
}
}
Expand Down
15 changes: 10 additions & 5 deletions crates/oxc_linter/src/rules/nextjs/no_unwanted_polyfillio.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use cow_utils::CowUtils;
use oxc_ast::{
ast::{JSXAttributeItem, JSXAttributeName, JSXAttributeValue},
AstKind,
Expand All @@ -8,7 +9,11 @@ use oxc_semantic::AstNode;
use oxc_span::Span;
use phf::{phf_set, Set};

use crate::{context::LintContext, rule::Rule, utils::get_next_script_import_local_name};
use crate::{
context::LintContext,
rule::Rule,
utils::{find_url_query_value, get_next_script_import_local_name},
};

fn no_unwanted_polyfillio_diagnostic(polyfill_name: &str, span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn(format!(
Expand Down Expand Up @@ -147,14 +152,14 @@ impl Rule for NoUnwantedPolyfillio {
if src_value.value.as_str().starts_with("https://cdn.polyfill.io/v2/")
|| src_value.value.as_str().starts_with("https://polyfill.io/v3/")
{
let Ok(url) = url::Url::parse(src_value.value.as_str()) else {
return;
};
let Some((_, features_value)) = url.query_pairs().find(|(key, _)| key == "features")
let Some(features_value) = find_url_query_value(src_value.value.as_str(), "features")
else {
return;
};

// Replace URL encoded values
let features_value = features_value.cow_replace("%2C", ",");

let unwanted_features: Vec<&str> = features_value
.split(',')
.filter(|feature| NEXT_POLYFILLED_FEATURES.contains(feature))
Expand Down
3 changes: 2 additions & 1 deletion crates/oxc_linter/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ mod promise;
mod react;
mod react_perf;
mod unicorn;
mod url;
mod vitest;

use std::{io, path::Path};

pub use self::{
config::*, express::*, jest::*, jsdoc::*, nextjs::*, promise::*, react::*, react_perf::*,
unicorn::*, vitest::*,
unicorn::*, url::*, vitest::*,
};

/// List of Jest rules that have Vitest equivalents.
Expand Down
49 changes: 49 additions & 0 deletions crates/oxc_linter/src/utils/url.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/// Finds the first value of a query parameter in a URL. Is not guaranteed to be accurate
/// with the URL standard and is just meant to be a simple helper that doesn't require
/// fully parsing the URL.
///
/// # Example
///
/// ```rust
/// find_url_query_value("https://example.com/?foo=bar&baz=qux", "baz") // => Some("qux")
/// ```
pub fn find_url_query_value<'url>(url: &'url str, key: &str) -> Option<&'url str> {
// Return None right away if this doesn't look like a URL at all.
if !url.starts_with("http://") && !url.starts_with("https://") {
return None;
}
// Skip everything up to the first `?` as we're not parsing the host/path/etc.
let url = url.split('?').nth(1)?;
// Now parse the query string in pairs of `key=value`, we don't need
// to be too strict about this as we're not trying to be spec-compliant.
for pair in url.split('&') {
if let Some((k, v)) = pair.split_once('=') {
if k == key {
return Some(v);
}
}
}
None
}

mod test {
#[test]
fn test_find_url_query_value() {
use super::find_url_query_value;
assert_eq!(find_url_query_value("something", "q"), None);
assert_eq!(find_url_query_value("https://example.com/?foo=bar", "foo"), Some("bar"));
assert_eq!(find_url_query_value("https://example.com/?foo=bar", "baz"), None);
assert_eq!(
find_url_query_value("https://example.com/?foo=bar&baz=qux", "baz"),
Some("qux")
);
assert_eq!(
find_url_query_value("https://example.com/?foo=bar&foo=qux", "foo"),
Some("bar")
);
assert_eq!(
find_url_query_value("https://polyfill.io/v3/polyfill.min.js?features=WeakSet%2CPromise%2CPromise.prototype.finally%2Ces2015%2Ces5%2Ces6", "features"),
Some("WeakSet%2CPromise%2CPromise.prototype.finally%2Ces2015%2Ces5%2Ces6")
);
}
}
1 change: 0 additions & 1 deletion tasks/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,3 @@ project-root = { workspace = true }
similar = { workspace = true }

ureq = { workspace = true, features = ["json", "rustls"] }
url = { workspace = true }
12 changes: 6 additions & 6 deletions tasks/common/src/test_file.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{fmt, str::FromStr};
use std::fmt;

use crate::{project_root, request::agent};

Expand Down Expand Up @@ -94,11 +94,11 @@ impl TestFile {
/// # Errors
/// # Panics
pub fn get_source_text(lib: &str) -> Result<(String, String), String> {
let url = url::Url::from_str(lib).map_err(err_to_string)?;

let segments = url.path_segments().ok_or_else(|| "lib url has no segments".to_string())?;

let filename = segments.last().ok_or_else(|| "lib url has no segments".to_string())?;
if !lib.starts_with("https://") {
return Err(format!("Not an https url: {lib:?}"));
}
let filename =
lib.split('/').last().ok_or_else(|| "lib url has no segments".to_string())?;

let file = project_root().join("target").join(filename);

Expand Down

0 comments on commit 325753b

Please sign in to comment.