Skip to content

Commit

Permalink
chore: extract noPackagePrivateImports rule (#4651)
Browse files Browse the repository at this point in the history
  • Loading branch information
arendjr authored Nov 28, 2024
1 parent f6009f0 commit d8f91ba
Show file tree
Hide file tree
Showing 16 changed files with 366 additions and 387 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
cli: major
---

# The rule `useImportRestrictions` has been renamed to `noPackagePrivateImports`

To avoid confusion with `noRestrictedImports`, `useImportRestrictions` has been
renamed to `noPackagePrivateImports`.

This file was deleted.

154 changes: 87 additions & 67 deletions crates/biome_configuration/src/analyzer/linter/rules.rs

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions crates/biome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ define_categories! {
"lint/nursery/noMissingVarFunction": "https://biomejs.dev/linter/rules/no-missing-var-function",
"lint/nursery/noNestedTernary": "https://biomejs.dev/linter/rules/no-nested-ternary",
"lint/nursery/noOctalEscape": "https://biomejs.dev/linter/rules/no-octal-escape",
"lint/nursery/noPackagePrivateImports": "https://biomejs.dev/linter/rules/no-package-private-imports",
"lint/nursery/noProcessEnv": "https://biomejs.dev/linter/rules/no-process-env",
"lint/nursery/noReactSpecificProps": "https://biomejs.dev/linter/rules/no-react-specific-props",
"lint/nursery/noRestrictedImports": "https://biomejs.dev/linter/rules/no-restricted-imports",
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 @@ -16,6 +16,7 @@ pub mod no_img_element;
pub mod no_irregular_whitespace;
pub mod no_nested_ternary;
pub mod no_octal_escape;
pub mod no_package_private_imports;
pub mod no_process_env;
pub mod no_restricted_imports;
pub mod no_restricted_types;
Expand Down Expand Up @@ -60,6 +61,7 @@ declare_lint_group! {
self :: no_irregular_whitespace :: NoIrregularWhitespace ,
self :: no_nested_ternary :: NoNestedTernary ,
self :: no_octal_escape :: NoOctalEscape ,
self :: no_package_private_imports :: NoPackagePrivateImports ,
self :: no_process_env :: NoProcessEnv ,
self :: no_restricted_imports :: NoRestrictedImports ,
self :: no_restricted_types :: NoRestrictedTypes ,
Expand Down
199 changes: 199 additions & 0 deletions crates/biome_js_analyze/src/lint/nursery/no_package_private_imports.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
use biome_analyze::{
context::RuleContext, declare_lint_rule, Ast, Rule, RuleDiagnostic, RuleSource,
};
use biome_console::markup;
use biome_js_syntax::{inner_string_text, AnyJsImportLike};
use biome_rowan::{TextRange, TokenText};

const INDEX_BASENAMES: &[&str] = &["index", "mod"];

const SOURCE_EXTENSIONS: &[&str] = &["js", "ts", "cjs", "cts", "mjs", "mts", "jsx", "tsx"];

declare_lint_rule! {
/// Restricts imports of "package private" exports.
///
/// By enabling this rule, 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. External dependencies
/// as well as TypeScript aliases are exempted.
/// * This rule only applies to imports for JavaScript and TypeScript
/// files. Imports for resources such as images or CSS files are exempted.
///
/// Source: https://github.com/uhyo/eslint-plugin-import-access
///
/// #### Examples (Invalid)
///
/// ```js,expect_diagnostic
/// // Attempt to import from `foo.js` from outside its `sub` module.
/// import { fooPackageVariable } from "./sub/foo.js";
/// ```
/// ```js,expect_diagnostic
/// // Attempt to import from `bar.ts` from outside its `aunt` module.
/// import { barPackageVariable } from "../aunt/bar.ts";
/// ```
///
/// ```js,expect_diagnostic
/// // Assumed to resolve to a JS/TS file.
/// import { fooPackageVariable } from "./sub/foo";
/// ```
///
/// ```js,expect_diagnostic
/// // If the `sub/foo` module is inaccessible, so is its index file.
/// import { fooPackageVariable } from "./sub/foo/index.js";
/// ```
///
/// #### Examples (Valid)
///
/// ```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";
///
/// // A parent index file is accessible like other modules.
/// import { internal } from "../../index.js";
///
/// // If the `sub` module is accessible, so is its index file.
/// import { subPackageVariable } from "./sub/index.js";
///
/// // Library imports are exempt.
/// import useAsync from "react-use/lib/useAsync";
/// ```
///
pub NoPackagePrivateImports {
version: "next",
name: "noPackagePrivateImports",
language: "js",
sources: &[
RuleSource::EslintImportAccess("eslint-plugin-import-access")
],
recommended: false,
}
}

pub struct NoPackagePrivateImportsState {
range: TextRange,

/// The path that is being restricted.
path: String,

/// Suggestion from which to import instead.
suggestion: String,
}

impl Rule for NoPackagePrivateImports {
type Query = Ast<AnyJsImportLike>;
type State = NoPackagePrivateImportsState;
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();
if node.is_in_ts_module_declaration() {
return None;
}

let module_name = node.module_name_token()?;
let import_source_text = inner_string_text(&module_name);

get_restricted_import(module_name.text_trimmed_range(), &import_source_text)
}

fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let diagnostic = RuleDiagnostic::new(
rule_category!(),
state.range,
markup! {
"Importing package private symbols is disallowed from outside the module directory."
},
)
.note(markup! {
"Please import from "<Emphasis>{state.suggestion}</Emphasis>" instead "
"(you may need to re-export the symbol(s) from "<Emphasis>{state.path}</Emphasis>")."
});

Some(diagnostic)
}
}

fn get_restricted_import(
range: TextRange,
module_path: &TokenText,
) -> Option<NoPackagePrivateImportsState> {
if !module_path.starts_with('.') {
return None;
}

let mut path_parts: Vec<_> = module_path.text().split('/').collect();
let mut index_filename = None;

// TODO. The implementation could be optimized further by using
// `Path::new(module_path.text())` for further inspiration see `use_import_extensions` rule.
if let Some(extension) = get_extension(&path_parts) {
if !SOURCE_EXTENSIONS.contains(&extension) {
return None; // Resource files are exempt.
}

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_parts.last().copied();
path_parts.pop();
}
}
}

let is_restricted = path_parts
.iter()
.filter(|&&part| part != "." && part != "..")
.count()
> 1;
if !is_restricted {
return None;
}

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_parts.push(index_filename);

// Assumes the user probably wants to use an index file that has the
// same name as the original.
suggestion_parts.push(index_filename);
}

Some(NoPackagePrivateImportsState {
range,
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,
})
}
Loading

0 comments on commit d8f91ba

Please sign in to comment.