Skip to content

Commit f6a62e5

Browse files
committed
WIP: noPrivateImports
1 parent c7703a4 commit f6a62e5

14 files changed

+532
-321
lines changed

.changeset/renamed_useimportrestrictions_to_nopackageprivateimports.md

-8
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"@biomejs/biome": major
3+
---
4+
5+
The rule `useImportRestrictions` has been renamed to `noPrivateImports`, and its
6+
functionality has been significantly upgraded.
7+
8+
Previously, the rule would assume that any direct imports from modules inside
9+
other directories should be forbidden due to their _package private_ visibility.
10+
11+
The updated rule allows configuring the default visibility of exports, and
12+
recognises JSDoc comments to override this visibility. The default visibility
13+
is now `**public**`, but can be set to `**package**`, or even `**private**`.
14+
15+
`noPrivateImports` is now recommended by default.

crates/biome_dependency_graph/src/dependency_graph.rs

+157-78
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,18 @@
55
//!
66
//! The dependency graph is instantiated and updated inside the Workspace
77
//! Server.
8-
use std::{collections::BTreeMap, sync::Arc};
8+
use std::{
9+
collections::{BTreeMap, BTreeSet},
10+
sync::Arc,
11+
};
912

1013
use biome_fs::{BiomePath, FileSystem, PathKind};
11-
use biome_js_syntax::AnyJsRoot;
14+
use biome_js_syntax::{AnyJsImportLike, AnyJsRoot};
1215
use biome_project_layout::ProjectLayout;
1316
use biome_rowan::Text;
1417
use camino::{Utf8Path, Utf8PathBuf};
1518
use oxc_resolver::{EnforceExtension, ResolveError, ResolveOptions, ResolverGeneric};
16-
use papaya::HashMap;
19+
use papaya::{HashMap, HashMapRef, LocalGuard};
1720
use rustc_hash::FxBuildHasher;
1821

1922
use crate::{module_visitor::ModuleVisitor, resolver_cache::ResolverCache};
@@ -33,81 +36,6 @@ pub struct DependencyGraph {
3336
path_info: HashMap<Utf8PathBuf, Option<PathKind>>,
3437
}
3538

36-
#[derive(Clone, Debug, Default)]
37-
pub struct ModuleDependencyData {
38-
/// Map of all static imports found in the module.
39-
///
40-
/// Maps from the identifier found in the import statement to the absolute
41-
/// path it resolves to. The resolved path may be looked up as key in the
42-
/// [DependencyGraphModel::modules] map, although it is not required to
43-
/// exist (for instance, if the path is outside the project's scope).
44-
///
45-
/// Note that re-exports may introduce additional dependencies, because they
46-
/// import another module and immediately re-export from that module.
47-
/// Re-exports are tracked as part of [Self::exports] and
48-
/// [Self::blanket_reexports].
49-
pub static_imports: BTreeMap<String, Import>,
50-
51-
/// Map of all dynamic imports found in the module for which the import
52-
/// identifier could be statically determined.
53-
///
54-
/// Dynamic imports for which the identifier cannot be statically determined
55-
/// (for instance, because a template string with variables is used) will be
56-
/// omitted from this map.
57-
///
58-
/// Maps from the identifier found in the import expression to the absolute
59-
/// path it resolves to. The resolved path may be looked up as key in the
60-
/// [DependencyGraphModel::modules] map, although it is not required to
61-
/// exist (for instance, if the path is outside the project's scope).
62-
///
63-
/// `require()` expressions in CommonJS sources are also included with the
64-
/// dynamic imports.
65-
pub dynamic_imports: BTreeMap<String, Import>,
66-
67-
/// Map of exports from the module.
68-
///
69-
/// The keys are the names of the exports, where "default" is used for the
70-
/// default export. See [Export] for information tracked per export.
71-
///
72-
/// Re-exports are tracked in this map as well. They exception are "blanket"
73-
/// re-exports, such as `export * from "other-module"`. Those are tracked in
74-
/// [Self::forwarding_exports] instead.
75-
pub exports: BTreeMap<Text, Export>,
76-
77-
/// Re-exports that apply to all symbols from another module, without
78-
/// assigning a name to them.
79-
pub blanket_reexports: Vec<ReexportAll>,
80-
}
81-
82-
impl ModuleDependencyData {
83-
/// Allows draining a single entry from the imports.
84-
///
85-
/// Returns a `(specifier, import)` pair from either the static or dynamic
86-
/// imports, whichever is non-empty. Returns `None` if both are empty.
87-
///
88-
/// Using this method allows for consuming the struct while iterating over
89-
/// it, without necessarily turning the entire struct into an iterator at
90-
/// once.
91-
pub fn drain_one(&mut self) -> Option<(String, Import)> {
92-
if self.static_imports.is_empty() {
93-
self.dynamic_imports.pop_first()
94-
} else {
95-
self.static_imports.pop_first()
96-
}
97-
}
98-
}
99-
100-
#[derive(Clone, Debug, PartialEq)]
101-
pub struct Import {
102-
/// Absolute path of the resource being imported, if it can be resolved.
103-
///
104-
/// If the import statement referred to a package dependency, the path will
105-
/// point towards the resolved entry point of the package.
106-
///
107-
/// If `None`, import resolution failed.
108-
pub resolved_path: Result<Utf8PathBuf, ResolveError>,
109-
}
110-
11139
impl DependencyGraph {
11240
/// Returns the dependency data, such as imports and exports, for the
11341
/// given `path`.
@@ -207,6 +135,157 @@ impl DependencyGraph {
207135
pub(crate) fn path_kind(&self, path: &Utf8Path) -> Option<PathKind> {
208136
self.path_info.pin().get(path).copied().flatten()
209137
}
138+
139+
/// Finds an exported symbol by `symbol_name` as exported by `module`.
140+
///
141+
/// Follows re-exports if necessary.
142+
fn find_exported_symbol(
143+
&self,
144+
module: &ModuleDependencyData,
145+
symbol_name: &str,
146+
) -> Option<OwnExport> {
147+
let data = self.data.pin();
148+
let mut seen_paths = BTreeSet::new();
149+
150+
fn find_exported_symbol_with_seen_paths<'a>(
151+
data: &'a HashMapRef<Utf8PathBuf, ModuleDependencyData, FxBuildHasher, LocalGuard>,
152+
module: &'a ModuleDependencyData,
153+
symbol_name: &str,
154+
seen_paths: &mut BTreeSet<&'a Utf8Path>,
155+
) -> Option<OwnExport> {
156+
match module.exports.get(symbol_name) {
157+
Some(Export::Own(own_export)) => Some(own_export.clone()),
158+
Some(Export::Reexport(import)) => match &import.resolved_path {
159+
Ok(path) if seen_paths.insert(path) => data.get(path).and_then(|module| {
160+
find_exported_symbol_with_seen_paths(data, module, symbol_name, seen_paths)
161+
}),
162+
_ => None,
163+
},
164+
// FIXME: We can create an `OwnExport` on the fly from a
165+
// `ReexportAll`, which sorta kinda makes sense,
166+
// because it does export the imported symbols under
167+
// its own name.
168+
// Still, it feels funny, and it means we always
169+
// ignore JSDoc comments added to such a re-export.
170+
// Should be fixed as part of #5312.
171+
Some(Export::ReexportAll(_)) => Some(OwnExport {
172+
jsdoc_comment: None,
173+
}),
174+
None => module.blanket_reexports.iter().find_map(|reexport| {
175+
match &reexport.import.resolved_path {
176+
Ok(path) if seen_paths.insert(path) => data.get(path).and_then(|module| {
177+
find_exported_symbol_with_seen_paths(
178+
data,
179+
module,
180+
symbol_name,
181+
seen_paths,
182+
)
183+
}),
184+
_ => None,
185+
}
186+
}),
187+
}
188+
}
189+
190+
find_exported_symbol_with_seen_paths(&data, module, symbol_name, &mut seen_paths)
191+
}
192+
}
193+
194+
#[derive(Clone, Debug, Default)]
195+
pub struct ModuleDependencyData {
196+
/// Map of all static imports found in the module.
197+
///
198+
/// Maps from the identifier found in the import statement to the absolute
199+
/// path it resolves to. The resolved path may be looked up as key in the
200+
/// [DependencyGraphModel::modules] map, although it is not required to
201+
/// exist (for instance, if the path is outside the project's scope).
202+
///
203+
/// Note that re-exports may introduce additional dependencies, because they
204+
/// import another module and immediately re-export from that module.
205+
/// Re-exports are tracked as part of [Self::exports] and
206+
/// [Self::blanket_reexports].
207+
pub static_imports: BTreeMap<String, Import>,
208+
209+
/// Map of all dynamic imports found in the module for which the import
210+
/// identifier could be statically determined.
211+
///
212+
/// Dynamic imports for which the identifier cannot be statically determined
213+
/// (for instance, because a template string with variables is used) will be
214+
/// omitted from this map.
215+
///
216+
/// Maps from the identifier found in the import expression to the absolute
217+
/// path it resolves to. The resolved path may be looked up as key in the
218+
/// [DependencyGraphModel::modules] map, although it is not required to
219+
/// exist (for instance, if the path is outside the project's scope).
220+
///
221+
/// `require()` expressions in CommonJS sources are also included with the
222+
/// dynamic imports.
223+
pub dynamic_imports: BTreeMap<String, Import>,
224+
225+
/// Map of exports from the module.
226+
///
227+
/// The keys are the names of the exports, where "default" is used for the
228+
/// default export. See [Export] for information tracked per export.
229+
///
230+
/// Re-exports are tracked in this map as well. They exception are "blanket"
231+
/// re-exports, such as `export * from "other-module"`. Those are tracked in
232+
/// [Self::forwarding_exports] instead.
233+
pub exports: BTreeMap<Text, Export>,
234+
235+
/// Re-exports that apply to all symbols from another module, without
236+
/// assigning a name to them.
237+
pub blanket_reexports: Vec<ReexportAll>,
238+
}
239+
240+
impl ModuleDependencyData {
241+
/// Allows draining a single entry from the imports.
242+
///
243+
/// Returns a `(specifier, import)` pair from either the static or dynamic
244+
/// imports, whichever is non-empty. Returns `None` if both are empty.
245+
///
246+
/// Using this method allows for consuming the struct while iterating over
247+
/// it, without necessarily turning the entire struct into an iterator at
248+
/// once.
249+
pub fn drain_one(&mut self) -> Option<(String, Import)> {
250+
if self.static_imports.is_empty() {
251+
self.dynamic_imports.pop_first()
252+
} else {
253+
self.static_imports.pop_first()
254+
}
255+
}
256+
257+
/// Finds an exported symbol by `name`, using the `dependency_graph` to
258+
/// lookup re-exports if necessary.
259+
#[inline]
260+
pub fn find_exported_symbol(
261+
&self,
262+
dependency_graph: &DependencyGraph,
263+
name: &str,
264+
) -> Option<OwnExport> {
265+
dependency_graph.find_exported_symbol(self, name)
266+
}
267+
268+
/// Returns the information about a given import by its syntax node.
269+
pub fn get_import_by_node(&self, node: &AnyJsImportLike) -> Option<&Import> {
270+
let specifier_text = node.inner_string_text()?;
271+
let specifier = specifier_text.text();
272+
if node.is_static_import() {
273+
self.static_imports.get(specifier)
274+
} else {
275+
self.dynamic_imports.get(specifier)
276+
}
277+
}
278+
}
279+
280+
#[derive(Clone, Debug, PartialEq)]
281+
pub struct Import {
282+
/// Absolute path of the resource being imported, if it can be resolved.
283+
///
284+
/// If the import statement referred to a package dependency, the path will
285+
/// point towards the resolved entry point of the package.
286+
///
287+
/// If `None`, import resolution failed.
288+
pub resolved_path: Result<Utf8PathBuf, ResolveError>,
210289
}
211290

212291
/// Information tracked for every export.

crates/biome_dependency_graph/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@ mod dependency_graph;
22
mod module_visitor;
33
mod resolver_cache;
44

5-
pub use dependency_graph::{DependencyGraph, ModuleDependencyData};
5+
pub use dependency_graph::{DependencyGraph, Import, ModuleDependencyData};

crates/biome_js_analyze/src/lint/correctness.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ pub mod no_new_symbol;
2121
pub mod no_nodejs_modules;
2222
pub mod no_nonoctal_decimal_escape;
2323
pub mod no_precision_loss;
24+
pub mod no_private_imports;
2425
pub mod no_render_return_value;
2526
pub mod no_self_assign;
2627
pub mod no_setter_return;
@@ -48,4 +49,4 @@ pub mod use_is_nan;
4849
pub mod use_jsx_key_in_iterable;
4950
pub mod use_valid_for_direction;
5051
pub mod use_yield;
51-
declare_lint_group! { pub Correctness { name : "correctness" , rules : [self :: no_children_prop :: NoChildrenProp , self :: no_const_assign :: NoConstAssign , self :: no_constant_condition :: NoConstantCondition , self :: no_constant_math_min_max_clamp :: NoConstantMathMinMaxClamp , self :: no_constructor_return :: NoConstructorReturn , self :: no_empty_character_class_in_regex :: NoEmptyCharacterClassInRegex , self :: no_empty_pattern :: NoEmptyPattern , self :: no_flat_map_identity :: NoFlatMapIdentity , self :: no_global_object_calls :: NoGlobalObjectCalls , self :: no_inner_declarations :: NoInnerDeclarations , self :: no_invalid_builtin_instantiation :: NoInvalidBuiltinInstantiation , self :: no_invalid_constructor_super :: NoInvalidConstructorSuper , self :: no_invalid_new_builtin :: NoInvalidNewBuiltin , self :: no_invalid_use_before_declaration :: NoInvalidUseBeforeDeclaration , self :: no_new_symbol :: NoNewSymbol , self :: no_nodejs_modules :: NoNodejsModules , self :: no_nonoctal_decimal_escape :: NoNonoctalDecimalEscape , self :: no_precision_loss :: NoPrecisionLoss , self :: no_render_return_value :: NoRenderReturnValue , self :: no_self_assign :: NoSelfAssign , self :: no_setter_return :: NoSetterReturn , self :: no_string_case_mismatch :: NoStringCaseMismatch , self :: no_switch_declarations :: NoSwitchDeclarations , self :: no_undeclared_dependencies :: NoUndeclaredDependencies , self :: no_undeclared_variables :: NoUndeclaredVariables , self :: no_unnecessary_continue :: NoUnnecessaryContinue , self :: no_unreachable :: NoUnreachable , self :: no_unreachable_super :: NoUnreachableSuper , self :: no_unsafe_finally :: NoUnsafeFinally , self :: no_unsafe_optional_chaining :: NoUnsafeOptionalChaining , self :: no_unused_function_parameters :: NoUnusedFunctionParameters , self :: no_unused_imports :: NoUnusedImports , self :: no_unused_labels :: NoUnusedLabels , self :: no_unused_private_class_members :: NoUnusedPrivateClassMembers , self :: no_unused_variables :: NoUnusedVariables , self :: no_void_elements_with_children :: NoVoidElementsWithChildren , self :: no_void_type_return :: NoVoidTypeReturn , self :: use_array_literals :: UseArrayLiterals , self :: use_exhaustive_dependencies :: UseExhaustiveDependencies , self :: use_hook_at_top_level :: UseHookAtTopLevel , self :: use_import_extensions :: UseImportExtensions , self :: use_is_nan :: UseIsNan , self :: use_jsx_key_in_iterable :: UseJsxKeyInIterable , self :: use_valid_for_direction :: UseValidForDirection , self :: use_yield :: UseYield ,] } }
52+
declare_lint_group! { pub Correctness { name : "correctness" , rules : [self :: no_children_prop :: NoChildrenProp , self :: no_const_assign :: NoConstAssign , self :: no_constant_condition :: NoConstantCondition , self :: no_constant_math_min_max_clamp :: NoConstantMathMinMaxClamp , self :: no_constructor_return :: NoConstructorReturn , self :: no_empty_character_class_in_regex :: NoEmptyCharacterClassInRegex , self :: no_empty_pattern :: NoEmptyPattern , self :: no_flat_map_identity :: NoFlatMapIdentity , self :: no_global_object_calls :: NoGlobalObjectCalls , self :: no_inner_declarations :: NoInnerDeclarations , self :: no_invalid_builtin_instantiation :: NoInvalidBuiltinInstantiation , self :: no_invalid_constructor_super :: NoInvalidConstructorSuper , self :: no_invalid_new_builtin :: NoInvalidNewBuiltin , self :: no_invalid_use_before_declaration :: NoInvalidUseBeforeDeclaration , self :: no_new_symbol :: NoNewSymbol , self :: no_nodejs_modules :: NoNodejsModules , self :: no_nonoctal_decimal_escape :: NoNonoctalDecimalEscape , self :: no_private_imports :: NoPrivateImports , self :: no_precision_loss :: NoPrecisionLoss , self :: no_render_return_value :: NoRenderReturnValue , self :: no_self_assign :: NoSelfAssign , self :: no_setter_return :: NoSetterReturn , self :: no_string_case_mismatch :: NoStringCaseMismatch , self :: no_switch_declarations :: NoSwitchDeclarations , self :: no_undeclared_dependencies :: NoUndeclaredDependencies , self :: no_undeclared_variables :: NoUndeclaredVariables , self :: no_unnecessary_continue :: NoUnnecessaryContinue , self :: no_unreachable :: NoUnreachable , self :: no_unreachable_super :: NoUnreachableSuper , self :: no_unsafe_finally :: NoUnsafeFinally , self :: no_unsafe_optional_chaining :: NoUnsafeOptionalChaining , self :: no_unused_function_parameters :: NoUnusedFunctionParameters , self :: no_unused_imports :: NoUnusedImports , self :: no_unused_labels :: NoUnusedLabels , self :: no_unused_private_class_members :: NoUnusedPrivateClassMembers , self :: no_unused_variables :: NoUnusedVariables , self :: no_void_elements_with_children :: NoVoidElementsWithChildren , self :: no_void_type_return :: NoVoidTypeReturn , self :: use_array_literals :: UseArrayLiterals , self :: use_exhaustive_dependencies :: UseExhaustiveDependencies , self :: use_hook_at_top_level :: UseHookAtTopLevel , self :: use_import_extensions :: UseImportExtensions , self :: use_is_nan :: UseIsNan , self :: use_jsx_key_in_iterable :: UseJsxKeyInIterable , self :: use_valid_for_direction :: UseValidForDirection , self :: use_yield :: UseYield ,] } }

0 commit comments

Comments
 (0)