From f60d42d358b376437aa693004a319c409d5588ff Mon Sep 17 00:00:00 2001 From: Arend van Beelen jr Date: Tue, 18 Jul 2023 20:32:51 +0200 Subject: [PATCH] PR feedback --- Cargo.toml | 38 +++--- crates/rome_console/src/fmt.rs | 12 -- .../nursery/use_import_restrictions.rs | 110 +++++++++--------- .../invalidPackagePrivateImports.js.snap | 8 +- crates/rome_js_unicode_table/src/tables.rs | 8 +- .../src/configuration/linter/rules.rs | 53 ++++++--- .../src/configuration/parse/json/rules.rs | 31 ++--- editors/vscode/configuration_schema.json | 7 ++ npm/backend-jsonrpc/src/workspace.ts | 7 +- npm/rome/configuration_schema.json | 10 +- .../components/generated/NumberOfRules.astro | 2 +- website/src/pages/lint/rules/index.mdx | 6 + .../pages/lint/rules/useImportRestrictions.md | 35 ++---- 13 files changed, 174 insertions(+), 153 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1985afb3a61..138a7f74b94 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,9 @@ rome_aria_metadata = { path = "./crates/rome_aria_metadata" } rome_cli = { path = "./crates/rome_cli" } rome_console = { version = "0.0.1", path = "./crates/rome_console" } rome_control_flow = { path = "./crates/rome_control_flow" } +rome_css_factory = { path = "./crates/rome_css_factory" } +rome_css_parser = { path = "./crates/rome_css_parser" } +rome_css_syntax = { path = "./crates/rome_css_syntax" } rome_deserialize = { version = "0.0.0", path = "./crates/rome_deserialize" } rome_diagnostics = { version = "0.0.1", path = "./crates/rome_diagnostics" } rome_diagnostics_categories = { version = "0.0.1", path = "./crates/rome_diagnostics_categories" } @@ -51,9 +54,6 @@ rome_json_factory = { version = "0.0.1", path = "./crates/rome_json_fa rome_json_formatter = { path = "./crates/rome_json_formatter" } rome_json_parser = { path = "./crates/rome_json_parser" } rome_json_syntax = { version = "0.0.1", path = "./crates/rome_json_syntax" } -rome_css_factory = { path = "./crates/rome_css_factory" } -rome_css_parser = { path = "./crates/rome_css_parser" } -rome_css_syntax = { path = "./crates/rome_css_syntax" } rome_lsp = { path = "./crates/rome_lsp" } rome_markup = { version = "0.0.1", path = "./crates/rome_markup" } rome_migrate = { path = "./crates/rome_migrate" } @@ -65,24 +65,24 @@ rome_text_size = { version = "0.0.1", path = "./crates/rome_text_si tests_macros = { path = "./crates/tests_macros" } # Crates needed in the workspace +bitflags = "2.3.1" +bpaf = { version = "0.9.2", features = ["derive"] } +countme = "3.0.1" +dashmap = "5.4.0" +indexmap = "1.9.3" +insta = "1.29.0" +lazy_static = "1.4.0" +quickcheck = "1.0.3" quickcheck_macros = "1.0.0" -quickcheck = "1.0.3" -bitflags = "2.3.1" -bpaf = { version = "0.9.2", features = ["derive"] } -countme = "3.0.1" -dashmap = "5.4.0" -indexmap = "1.9.3" -insta = "1.29.0" -lazy_static = "1.4.0" -quote = { version = "1.0.28" } -rustc-hash = "1.1.0" -schemars = { version = "0.8.12" } -serde = { version = "1.0.163", features = ["derive"], default-features = false } -serde_json = "1.0.96" -smallvec = { version = "1.10.0", features = ["union", "const_new"] } -tracing = { version = "0.1.37", default-features = false, features = ["std"] } +quote = { version = "1.0.28" } +rustc-hash = "1.1.0" +schemars = { version = "0.8.12" } +serde = { version = "1.0.163", features = ["derive"], default-features = false } +serde_json = "1.0.96" +smallvec = { version = "1.10.0", features = ["union", "const_new"] } +tracing = { version = "0.1.37", default-features = false, features = ["std"] } # pinning to version 1.18 to avoid multiple versions of windows-sys as dependency -tokio = { version = "~1.18.5" } +tokio = { version = "~1.18.5" } [profile.dev.package.rome_wasm] diff --git a/crates/rome_console/src/fmt.rs b/crates/rome_console/src/fmt.rs index 082d8b7011d..b4231ac5c62 100644 --- a/crates/rome_console/src/fmt.rs +++ b/crates/rome_console/src/fmt.rs @@ -176,18 +176,6 @@ impl<'a> Display for std::fmt::Arguments<'a> { } } -impl<'a> Display for std::path::Display<'a> { - fn fmt(&self, fmt: &mut Formatter) -> io::Result<()> { - let mut string = self.to_string(); - if cfg!(target_os = "windows") { - // Explicitly use forward slashes on Windows for consistency with - // how they are used in JS `import` statements. - string = string.replace('\\', "/"); - } - fmt.write_str(&string) - } -} - /// Implement [Display] for types that implement [std::fmt::Display] by calling /// through to [Formatter::write_fmt] macro_rules! impl_std_display { diff --git a/crates/rome_js_analyze/src/analyzers/nursery/use_import_restrictions.rs b/crates/rome_js_analyze/src/analyzers/nursery/use_import_restrictions.rs index 185d616986b..dea62e25452 100644 --- a/crates/rome_js_analyze/src/analyzers/nursery/use_import_restrictions.rs +++ b/crates/rome_js_analyze/src/analyzers/nursery/use_import_restrictions.rs @@ -2,43 +2,31 @@ use rome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic use rome_console::markup; use rome_js_syntax::JsModuleSource; use rome_rowan::{AstNode, SyntaxTokenText}; -use std::{ - ffi::OsStr, - path::{Path, PathBuf}, -}; const INDEX_BASENAMES: &[&str] = &["index", "mod"]; const SOURCE_EXTENSIONS: &[&str] = &["js", "ts", "cjs", "cts", "mjs", "mts", "jsx", "tsx"]; declare_rule! { - /// Disallows imports from certain modules. + /// Disallows package private imports. /// /// This rules enforces the following restrictions: /// /// ## Package private visibility /// - /// All exported symbols are considered to be "package private". This means that modules that - /// reside in the same directory, as well as submodules of those "sibling" modules, are - /// allowed to import them, while any other modules that are further away in the file system - /// are restricted from importing them. A symbol's visibility may be extended by - /// re-exporting from an index file. + /// All exported symbols, such as types, functions or other things that may be exported, are + /// considered to be "package private". This means that modules that reside in the same + /// directory, as well as submodules of those "sibling" modules, are allowed to import them, + /// while any other modules that are further away in the file system are restricted from + /// importing them. A symbol's visibility may be extended by re-exporting from an index file. /// /// Notes: /// - /// * This rule only applies to relative imports, since the API from external dependencies is - /// often out of your control. - /// * This rule only applies to source imports. Imports for resources such as images or CSS - /// files are exempted. - /// * A future improvement will relax the restriction from "all exported symbols" to those - /// that have an `@package` JSDoc annotation. + /// * This rule only applies to relative imports. External dependencies are exempted. + /// * This rule only applies to imports for JavaScript and TypeScript files. Imports for + /// resources such as images or CSS files are exempted. /// - /// This rule is intended to be extended with additional import restrictions. - /// Please see the tracking issue to follow progress: https://github.com/rome/tools/issues/4678 - /// - /// Source: - /// - /// * https://github.com/uhyo/eslint-plugin-import-access + /// Source: https://github.com/uhyo/eslint-plugin-import-access /// /// ## Examples /// @@ -64,9 +52,6 @@ declare_rule! { /// // Imports within the same module are always allowed. /// import { fooPackageVariable } from "./foo.js"; /// - /// // Imports within the same module are always allowed. - /// import { fooPackageVariable } from "./foo.js"; - /// /// // Resources (anything other than JS/TS files) are exempt. /// import { barResource } from "../aunt/bar.png"; /// @@ -105,20 +90,17 @@ impl Rule for UseImportRestrictions { fn diagnostic(ctx: &RuleContext, state: &Self::State) -> Option { let ImportRestrictionsState { path, suggestion } = state; - let mut diagnostic = RuleDiagnostic::new( + let diagnostic = RuleDiagnostic::new( rule_category!(), ctx.query().range(), markup! { - "Importing package private symbols is not allowed from outside the module directory." + "Importing package private symbols is prohibited from outside the module directory." }, - ); - - if let Some(suggestion) = suggestion { - diagnostic = diagnostic.note(markup! { - "Please import from "{suggestion.display()}" instead " - "(you may need to re-export the symbol(s) from "{path.display()}")." - }); - } + ) + .note(markup! { + "Please import from "{suggestion}" instead " + "(you may need to re-export the symbol(s) from "{path}")." + }); Some(diagnostic) } @@ -126,59 +108,75 @@ impl Rule for UseImportRestrictions { pub(crate) struct ImportRestrictionsState { /// The path that is being restricted. - path: PathBuf, + path: String, - /// Optional suggestion from which to import instead. - suggestion: Option, + /// Suggestion from which to import instead. + suggestion: String, } fn get_restricted_import(module_path: &SyntaxTokenText) -> Option { - let mut path = PathBuf::from(module_path.text()); - - if !path.starts_with(".") && !path.starts_with("..") { + if !module_path.starts_with('.') { return None; } + let mut path_parts: Vec<&str> = module_path.text().split('/').collect(); let mut index_filename = None; - if let Some(extension) = path.extension() { - if !SOURCE_EXTENSIONS.contains(&extension.to_str().unwrap_or_default()) { + if let Some(extension) = get_extension(&path_parts) { + if !SOURCE_EXTENSIONS.contains(&extension) { return None; // Resource files are exempt. } - if let Some(basename) = path.file_stem() { - if INDEX_BASENAMES.contains(&basename.to_str().unwrap_or_default()) { + if let Some(basename) = get_basename(&path_parts) { + if INDEX_BASENAMES.contains(&basename) { // We pop the index file because it shouldn't count as a path, // component, but we store the file name so we can add it to // both the reported path and the suggestion. - index_filename = path.file_name().map(OsStr::to_owned); - path.pop(); + index_filename = path_parts.last().cloned(); + path_parts.pop(); } } } - let is_restricted = path - .components() - .filter(|component| component.as_os_str() != "." && component.as_os_str() != "..") + let is_restricted = path_parts + .iter() + .filter(|&&part| part != "." && part != "..") .count() > 1; if !is_restricted { return None; } - let mut suggestion = path.parent().map(Path::to_owned); + let mut suggestion_parts = path_parts[..path_parts.len() - 1].to_vec(); // Push the index file if it exists. This makes sure the reported path // matches the import path exactly. if let Some(index_filename) = index_filename { - path.push(&index_filename); + path_parts.push(index_filename); // Assumes the user probably wants to use an index file that has the // same name as the original. - if let Some(alternative) = suggestion.as_mut() { - alternative.push(index_filename); - } + suggestion_parts.push(index_filename); } - Some(ImportRestrictionsState { path, suggestion }) + Some(ImportRestrictionsState { + path: path_parts.join("/"), + suggestion: suggestion_parts.join("/"), + }) +} + +fn get_basename<'a>(path_parts: &'_ [&'a str]) -> Option<&'a str> { + path_parts.last().map(|&part| match part.find('.') { + Some(dot_index) if dot_index > 0 && dot_index < part.len() - 1 => &part[..dot_index], + _ => part, + }) +} + +fn get_extension<'a>(path_parts: &'_ [&'a str]) -> Option<&'a str> { + path_parts.last().and_then(|part| match part.find('.') { + Some(dot_index) if dot_index > 0 && dot_index < part.len() - 1 => { + Some(&part[dot_index + 1..]) + } + _ => None, + }) } diff --git a/crates/rome_js_analyze/tests/specs/nursery/useImportRestrictions/invalidPackagePrivateImports.js.snap b/crates/rome_js_analyze/tests/specs/nursery/useImportRestrictions/invalidPackagePrivateImports.js.snap index ade437b391d..3e74a1033d0 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useImportRestrictions/invalidPackagePrivateImports.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/useImportRestrictions/invalidPackagePrivateImports.js.snap @@ -22,7 +22,7 @@ import { fooPackageVariable } from "./sub/foo/index.js"; ``` invalidPackagePrivateImports.js:2:36 lint/nursery/useImportRestrictions ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! Importing package private symbols is not allowed from outside the module directory. + ! Importing package private symbols is prohibited from outside the module directory. 1 │ // Attempt to import from `foo.js` from outside its `sub` module. > 2 │ import { fooPackageVariable } from "./sub/foo.js"; @@ -38,7 +38,7 @@ invalidPackagePrivateImports.js:2:36 lint/nursery/useImportRestrictions ━━ ``` invalidPackagePrivateImports.js:5:36 lint/nursery/useImportRestrictions ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! Importing package private symbols is not allowed from outside the module directory. + ! Importing package private symbols is prohibited from outside the module directory. 4 │ // Attempt to import from `bar.ts` from outside its `aunt` module. > 5 │ import { barPackageVariable } from "../aunt/bar.ts"; @@ -54,7 +54,7 @@ invalidPackagePrivateImports.js:5:36 lint/nursery/useImportRestrictions ━━ ``` invalidPackagePrivateImports.js:8:36 lint/nursery/useImportRestrictions ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! Importing package private symbols is not allowed from outside the module directory. + ! Importing package private symbols is prohibited from outside the module directory. 7 │ // Assumed to resolve to a JS/TS file. > 8 │ import { fooPackageVariable } from "./sub/foo"; @@ -70,7 +70,7 @@ invalidPackagePrivateImports.js:8:36 lint/nursery/useImportRestrictions ━━ ``` invalidPackagePrivateImports.js:11:36 lint/nursery/useImportRestrictions ━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! Importing package private symbols is not allowed from outside the module directory. + ! Importing package private symbols is prohibited from outside the module directory. 10 │ // If the `sub/foo` module is inaccessible, so is its index file. > 11 │ import { fooPackageVariable } from "./sub/foo/index.js"; diff --git a/crates/rome_js_unicode_table/src/tables.rs b/crates/rome_js_unicode_table/src/tables.rs index 863365853a9..3ed00b03214 100644 --- a/crates/rome_js_unicode_table/src/tables.rs +++ b/crates/rome_js_unicode_table/src/tables.rs @@ -787,7 +787,9 @@ pub mod derived_property { ('𱍐', '𲎯'), ('\u{e0100}', '\u{e01ef}'), ]; - pub fn ID_Continue(c: char) -> bool { super::bsearch_range_table(c, ID_Continue_table) } + pub fn ID_Continue(c: char) -> bool { + super::bsearch_range_table(c, ID_Continue_table) + } pub const ID_Start_table: &[(char, char)] = &[ ('A', 'Z'), ('a', 'z'), @@ -1449,5 +1451,7 @@ pub mod derived_property { ('𰀀', '𱍊'), ('𱍐', '𲎯'), ]; - pub fn ID_Start(c: char) -> bool { super::bsearch_range_table(c, ID_Start_table) } + pub fn ID_Start(c: char) -> bool { + super::bsearch_range_table(c, ID_Start_table) + } } diff --git a/crates/rome_service/src/configuration/linter/rules.rs b/crates/rome_service/src/configuration/linter/rules.rs index 4f35362386a..8d30ce78914 100644 --- a/crates/rome_service/src/configuration/linter/rules.rs +++ b/crates/rome_service/src/configuration/linter/rules.rs @@ -1959,6 +1959,15 @@ pub struct Nursery { #[bpaf(long("use-hook-at-top-level"), argument("on|off|warn"), optional, hide)] #[serde(skip_serializing_if = "Option::is_none")] pub use_hook_at_top_level: Option, + #[doc = "Disallows package private imports."] + #[bpaf( + long("use-import-restrictions"), + argument("on|off|warn"), + optional, + hide + )] + #[serde(skip_serializing_if = "Option::is_none")] + pub use_import_restrictions: Option, #[doc = "Use Array.isArray() instead of instanceof Array."] #[bpaf(long("use-is-array"), argument("on|off|warn"), optional, hide)] #[serde(skip_serializing_if = "Option::is_none")] @@ -1996,7 +2005,7 @@ pub struct Nursery { } impl Nursery { const GROUP_NAME: &'static str = "nursery"; - pub(crate) const GROUP_RULES: [&'static str; 34] = [ + pub(crate) const GROUP_RULES: [&'static str; 35] = [ "noAccumulatingSpread", "noAriaUnsupportedElements", "noBannedTypes", @@ -2025,6 +2034,7 @@ impl Nursery { "useGroupedTypeImport", "useHeadingContent", "useHookAtTopLevel", + "useImportRestrictions", "useIsArray", "useIsNan", "useLiteralEnumMembers", @@ -2074,8 +2084,9 @@ impl Nursery { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[29]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[30]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32]), ]; - const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 34] = [ + const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 35] = [ RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[0]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[1]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[2]), @@ -2110,6 +2121,7 @@ impl Nursery { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[33]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[34]), ]; #[doc = r" Retrieves the recommended rules"] pub(crate) fn is_recommended(&self) -> bool { matches!(self.recommended, Some(true)) } @@ -2260,36 +2272,41 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[27])); } } - if let Some(rule) = self.use_is_array.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[28])); } } - if let Some(rule) = self.use_is_nan.as_ref() { + if let Some(rule) = self.use_is_array.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[29])); } } - if let Some(rule) = self.use_literal_enum_members.as_ref() { + if let Some(rule) = self.use_is_nan.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[30])); } } - if let Some(rule) = self.use_literal_keys.as_ref() { + if let Some(rule) = self.use_literal_enum_members.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31])); } } - if let Some(rule) = self.use_naming_convention.as_ref() { + if let Some(rule) = self.use_literal_keys.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32])); } } - if let Some(rule) = self.use_simple_number_keys.as_ref() { + if let Some(rule) = self.use_naming_convention.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[33])); } } + if let Some(rule) = self.use_simple_number_keys.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[34])); + } + } index_set } pub(crate) fn get_disabled_rules(&self) -> IndexSet { @@ -2434,36 +2451,41 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[27])); } } - if let Some(rule) = self.use_is_array.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[28])); } } - if let Some(rule) = self.use_is_nan.as_ref() { + if let Some(rule) = self.use_is_array.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[29])); } } - if let Some(rule) = self.use_literal_enum_members.as_ref() { + if let Some(rule) = self.use_is_nan.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[30])); } } - if let Some(rule) = self.use_literal_keys.as_ref() { + if let Some(rule) = self.use_literal_enum_members.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31])); } } - if let Some(rule) = self.use_naming_convention.as_ref() { + if let Some(rule) = self.use_literal_keys.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32])); } } - if let Some(rule) = self.use_simple_number_keys.as_ref() { + if let Some(rule) = self.use_naming_convention.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[33])); } } + if let Some(rule) = self.use_simple_number_keys.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[34])); + } + } index_set } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] @@ -2475,7 +2497,7 @@ impl Nursery { pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 20] { Self::RECOMMENDED_RULES_AS_FILTERS } - pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 34] { Self::ALL_RULES_AS_FILTERS } + pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 35] { Self::ALL_RULES_AS_FILTERS } #[doc = r" Select preset rules"] pub(crate) fn collect_preset_rules( &self, @@ -2524,6 +2546,7 @@ impl Nursery { "useGroupedTypeImport" => self.use_grouped_type_import.as_ref(), "useHeadingContent" => self.use_heading_content.as_ref(), "useHookAtTopLevel" => self.use_hook_at_top_level.as_ref(), + "useImportRestrictions" => self.use_import_restrictions.as_ref(), "useIsArray" => self.use_is_array.as_ref(), "useIsNan" => self.use_is_nan.as_ref(), "useLiteralEnumMembers" => self.use_literal_enum_members.as_ref(), diff --git a/crates/rome_service/src/configuration/parse/json/rules.rs b/crates/rome_service/src/configuration/parse/json/rules.rs index 828a6d9e0e4..a68c3fc11f4 100644 --- a/crates/rome_service/src/configuration/parse/json/rules.rs +++ b/crates/rome_service/src/configuration/parse/json/rules.rs @@ -1672,11 +1672,8 @@ impl VisitNode for Nursery { "useGroupedTypeImport", "useHeadingContent", "useHookAtTopLevel", -<<<<<<< HEAD "useImportRestrictions", -======= "useIsArray", ->>>>>>> upstream/main "useIsNan", "useLiteralEnumMembers", "useLiteralKeys", @@ -2345,36 +2342,44 @@ impl VisitNode for Nursery { )); } }, -<<<<<<< HEAD "useImportRestrictions" => match value { AnyJsonValue::JsonStringValue(_) => { let mut configuration = RuleConfiguration::default(); self.map_to_known_string(&value, name_text, &mut configuration, diagnostics)?; self.use_import_restrictions = Some(configuration); -======= + } + AnyJsonValue::JsonObjectValue(_) => { + let mut rule_configuration = RuleConfiguration::default(); + rule_configuration.map_rule_configuration( + &value, + name_text, + "useImportRestrictions", + diagnostics, + )?; + self.use_import_restrictions = Some(rule_configuration); + } + _ => { + diagnostics.push(DeserializationDiagnostic::new_incorrect_type( + "object or string", + value.range(), + )); + } + }, "useIsArray" => match value { AnyJsonValue::JsonStringValue(_) => { let mut configuration = RuleConfiguration::default(); self.map_to_known_string(&value, name_text, &mut configuration, diagnostics)?; self.use_is_array = Some(configuration); ->>>>>>> upstream/main } AnyJsonValue::JsonObjectValue(_) => { let mut rule_configuration = RuleConfiguration::default(); rule_configuration.map_rule_configuration( &value, name_text, -<<<<<<< HEAD - "useImportRestrictions", - diagnostics, - )?; - self.use_import_restrictions = Some(rule_configuration); -======= "useIsArray", diagnostics, )?; self.use_is_array = Some(rule_configuration); ->>>>>>> upstream/main } _ => { diagnostics.push(DeserializationDiagnostic::new_incorrect_type( diff --git a/editors/vscode/configuration_schema.json b/editors/vscode/configuration_schema.json index c9e764d0910..85ec3879115 100644 --- a/editors/vscode/configuration_schema.json +++ b/editors/vscode/configuration_schema.json @@ -962,6 +962,13 @@ { "type": "null" } ] }, + "useImportRestrictions": { + "description": "Disallows package private imports.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "useIsArray": { "description": "Use Array.isArray() instead of instanceof Array.", "anyOf": [ diff --git a/npm/backend-jsonrpc/src/workspace.ts b/npm/backend-jsonrpc/src/workspace.ts index 68e5396a481..c51d2ec1272 100644 --- a/npm/backend-jsonrpc/src/workspace.ts +++ b/npm/backend-jsonrpc/src/workspace.ts @@ -627,15 +627,13 @@ export interface Nursery { */ useHookAtTopLevel?: RuleConfiguration; /** -<<<<<<< HEAD - * Disallows imports from certain modules. + * Disallows package private imports. */ useImportRestrictions?: RuleConfiguration; -======= + /** * Use Array.isArray() instead of instanceof Array. */ useIsArray?: RuleConfiguration; ->>>>>>> upstream/main /** * Require calls to isNaN() when checking for NaN. */ @@ -1179,6 +1177,7 @@ export type Category = | "lint/nursery/useGroupedTypeImport" | "lint/nursery/useHeadingContent" | "lint/nursery/useHookAtTopLevel" + | "lint/nursery/useImportRestrictions" | "lint/nursery/useIsArray" | "lint/nursery/useIsNan" | "lint/nursery/useLiteralEnumMembers" diff --git a/npm/rome/configuration_schema.json b/npm/rome/configuration_schema.json index 84800f06b4d..85ec3879115 100644 --- a/npm/rome/configuration_schema.json +++ b/npm/rome/configuration_schema.json @@ -962,13 +962,15 @@ { "type": "null" } ] }, -<<<<<<< HEAD "useImportRestrictions": { - "description": "Disallows imports from certain modules.", -======= + "description": "Disallows package private imports.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "useIsArray": { "description": "Use Array.isArray() instead of instanceof Array.", ->>>>>>> upstream/main "anyOf": [ { "$ref": "#/definitions/RuleConfiguration" }, { "type": "null" } diff --git a/website/src/components/generated/NumberOfRules.astro b/website/src/components/generated/NumberOfRules.astro index 8f19d322ea5..ea5c582e0f8 100644 --- a/website/src/components/generated/NumberOfRules.astro +++ b/website/src/components/generated/NumberOfRules.astro @@ -1,2 +1,2 @@ -

Rome's linter has a total of 153 rules

\ No newline at end of file +

Rome's linter has a total of 154 rules

\ No newline at end of file diff --git a/website/src/pages/lint/rules/index.mdx b/website/src/pages/lint/rules/index.mdx index 096d9f4b422..b844a5a3727 100644 --- a/website/src/pages/lint/rules/index.mdx +++ b/website/src/pages/lint/rules/index.mdx @@ -1067,6 +1067,12 @@ Enforce that all React hooks are being called from the Top Level component functions.

+

+ useImportRestrictions +

+Disallows package private imports. +
+

useIsArray

diff --git a/website/src/pages/lint/rules/useImportRestrictions.md b/website/src/pages/lint/rules/useImportRestrictions.md index e2023c9b301..f412b075409 100644 --- a/website/src/pages/lint/rules/useImportRestrictions.md +++ b/website/src/pages/lint/rules/useImportRestrictions.md @@ -5,33 +5,25 @@ parent: lint/rules/index # useImportRestrictions (since vnext) -Disallows imports from certain modules. +Disallows package private imports. This rules enforces the following restrictions: ## Package private visibility -All exported symbols are considered to be "package private". This means that modules that -reside in the same directory, as well as submodules of those "sibling" modules, are -allowed to import them, while any other modules that are further away in the file system -are restricted from importing them. A symbol's visibility may be extended by -re-exporting from an index file. +All exported symbols, such as types, functions or other things that may be exported, are +considered to be "package private". This means that modules that reside in the same +directory, as well as submodules of those "sibling" modules, are allowed to import them, +while any other modules that are further away in the file system are restricted from +importing them. A symbol's visibility may be extended by re-exporting from an index file. Notes: -- This rule only applies to relative imports, since the API from external dependencies is -often out of your control. -- This rule only applies to source imports. Imports for resources such as images or CSS -files are exempted. -- A future improvement will relax the restriction from "all exported symbols" to those -that have an `@package` JSDoc annotation. +- This rule only applies to relative imports. External dependencies are exempted. +- This rule only applies to imports for JavaScript and TypeScript files. Imports for +resources such as images or CSS files are exempted. -This rule is intended to be extended with additional import restrictions. -Please see the tracking issue to follow progress: https://github.com/rome/tools/issues/4678 - -Source: - -- https://github.com/uhyo/eslint-plugin-import-access +Source: https://github.com/uhyo/eslint-plugin-import-access ## Examples @@ -53,7 +45,7 @@ import { fooPackageVariable } from "./sub/foo/index.js"; 
nursery/useImportRestrictions.js:2:36 lint/nursery/useImportRestrictions ━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ⚠ Importing package private symbols is not allowed from outside the module directory.
+  ⚠ Importing package private symbols is prohibited from outside the module directory.
   
     1 │ // Attempt to import from `foo.js` from outside its `sub` module.
   > 2 │ import { fooPackageVariable } from "./sub/foo.js";
@@ -65,7 +57,7 @@ import { fooPackageVariable } from "./sub/foo/index.js";
   
 nursery/useImportRestrictions.js:5:36 lint/nursery/useImportRestrictions ━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ⚠ Importing package private symbols is not allowed from outside the module directory.
+  ⚠ Importing package private symbols is prohibited from outside the module directory.
   
     4 │ // Attempt to import from `bar.ts` from outside its `aunt` module.
   > 5 │ import { barPackageVariable } from "../aunt/bar.ts";
@@ -83,9 +75,6 @@ nursery/useImportRestrictions.js:5:36