Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: extract noPackagePrivateImports rule #4651

Merged
merged 1 commit into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since noRestrictedImports is a nursery rule, we can make this a patch, because nursery rules don't follow semver

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
Loading