From 6d5b697bec63d302e49cdd92f9c2f492ec98c385 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sat, 1 Feb 2025 18:55:47 -0500 Subject: [PATCH] refactor(linter): remove usage of `url` crate --- Cargo.lock | 2 - Cargo.toml | 1 - crates/oxc_linter/Cargo.toml | 1 - .../src/rules/nextjs/google_font_display.rs | 16 ++---- .../rules/nextjs/no_unwanted_polyfillio.rs | 15 ++++-- crates/oxc_linter/src/utils/mod.rs | 3 +- crates/oxc_linter/src/utils/url.rs | 49 +++++++++++++++++++ tasks/common/Cargo.toml | 1 - tasks/common/src/test_file.rs | 12 ++--- 9 files changed, 71 insertions(+), 29 deletions(-) create mode 100644 crates/oxc_linter/src/utils/url.rs diff --git a/Cargo.lock b/Cargo.lock index de6a5d1612e8d..170c66f1855d4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1848,7 +1848,6 @@ dependencies = [ "serde", "serde_json", "simdutf8", - "url", ] [[package]] @@ -2142,7 +2141,6 @@ dependencies = [ "project-root", "similar", "ureq", - "url", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index e585ca8143e66..a47df4006f7b7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" diff --git a/crates/oxc_linter/Cargo.toml b/crates/oxc_linter/Cargo.toml index 856aa64419750..237698f00b8f6 100644 --- a/crates/oxc_linter/Cargo.toml +++ b/crates/oxc_linter/Cargo.toml @@ -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 } diff --git a/crates/oxc_linter/src/rules/nextjs/google_font_display.rs b/crates/oxc_linter/src/rules/nextjs/google_font_display.rs index e0746872d05a8..f1be10cf2989d 100644 --- a/crates/oxc_linter/src/rules/nextjs/google_font_display.rs +++ b/crates/oxc_linter/src/rules/nextjs/google_font_display.rs @@ -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, }; @@ -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)); } } } diff --git a/crates/oxc_linter/src/rules/nextjs/no_unwanted_polyfillio.rs b/crates/oxc_linter/src/rules/nextjs/no_unwanted_polyfillio.rs index 3d7a4adc46b85..45255fcb93a35 100644 --- a/crates/oxc_linter/src/rules/nextjs/no_unwanted_polyfillio.rs +++ b/crates/oxc_linter/src/rules/nextjs/no_unwanted_polyfillio.rs @@ -1,3 +1,4 @@ +use cow_utils::CowUtils; use oxc_ast::{ ast::{JSXAttributeItem, JSXAttributeName, JSXAttributeValue}, AstKind, @@ -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!( @@ -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)) diff --git a/crates/oxc_linter/src/utils/mod.rs b/crates/oxc_linter/src/utils/mod.rs index c520ca4173377..aeab147c52504 100644 --- a/crates/oxc_linter/src/utils/mod.rs +++ b/crates/oxc_linter/src/utils/mod.rs @@ -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. diff --git a/crates/oxc_linter/src/utils/url.rs b/crates/oxc_linter/src/utils/url.rs new file mode 100644 index 0000000000000..3f9fa28e99a31 --- /dev/null +++ b/crates/oxc_linter/src/utils/url.rs @@ -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") + ); + } +} diff --git a/tasks/common/Cargo.toml b/tasks/common/Cargo.toml index cb4ba1e9c8a6e..afbe6515d0aac 100644 --- a/tasks/common/Cargo.toml +++ b/tasks/common/Cargo.toml @@ -19,4 +19,3 @@ project-root = { workspace = true } similar = { workspace = true } ureq = { workspace = true, features = ["json", "rustls"] } -url = { workspace = true } diff --git a/tasks/common/src/test_file.rs b/tasks/common/src/test_file.rs index 92fc71477cfc5..c555f855fda03 100644 --- a/tasks/common/src/test_file.rs +++ b/tasks/common/src/test_file.rs @@ -1,4 +1,4 @@ -use std::{fmt, str::FromStr}; +use std::fmt; use crate::{project_root, request::agent}; @@ -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);