Skip to content

Commit b43b704

Browse files
chore(rules_check): add multi file lint rule checking (#7399)
Co-authored-by: Naoki Ikeguchi <me@s6n.jp>
1 parent 0f38ea6 commit b43b704

File tree

5 files changed

+191
-18
lines changed

5 files changed

+191
-18
lines changed

Cargo.lock

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/biome_analyze/CONTRIBUTING.md

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,21 +1207,47 @@ The documentation needs to adhere to the following rules:
12071207

12081208
You should usually prefer to show a concise but complete sample snippet instead.
12091209

1210+
- **Multi-file snippets**
1211+
1212+
For rules that analyze relationships between multiple files (e.g., import cycles, cross-file dependencies), you can use the `file=<path>` property to create an in-memory file system for testing.
1213+
1214+
Files are organized by documentation section (Markdown headings), where all files in a section are collected before any tests run. This ensures each test has access to the complete file system regardless of definition order.
1215+
1216+
````rust
1217+
/// ### Invalid
1218+
///
1219+
/// **`foo.js`**
1220+
/// ```js,expect_diagnostic,file=foo.js
1221+
/// import { bar } from "./bar.js";
1222+
/// export function foo() {
1223+
/// return bar();
1224+
/// }
1225+
/// ```
1226+
///
1227+
/// **`bar.js`**
1228+
/// ```js,expect_diagnostic,file=bar.js
1229+
/// import { foo } from "./foo.js";
1230+
/// export function bar() {
1231+
/// return foo();
1232+
/// }
1233+
/// ```
1234+
````
1235+
12101236
- **Ordering of code block properties**
12111237

1212-
In addition to the language, a code block can be tagged with a few additional properties like `expect_diagnostic`, `options`, `full_options`, `use_options` and/or `ignore`.
1238+
In addition to the language, a code block can be tagged with a few additional properties like `expect_diagnostic`, `options`, `full_options`, `use_options`, `ignore` and/or `file=<path>`.
12131239

12141240
The parser does not care about the order, but for consistency, modifiers should always be ordered as follows:
12151241

12161242
````rust
1217-
/// ```<language>[,expect_diagnostic][,(options|full_options|use_options)][,ignore]
1243+
/// ```<language>[,expect_diagnostic][,(options|full_options|use_options)][,ignore][,file=path]
12181244
/// ```
12191245
````
12201246

12211247
e.g.
12221248

12231249
````rust
1224-
/// ```tsx,expect_diagnostic,use_options,ignore
1250+
/// ```tsx,expect_diagnostic,use_options,ignore,file=foobar.tsx
12251251
/// ```
12261252
````
12271253

crates/biome_js_analyze/src/lint/nursery/no_import_cycles.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ declare_lint_rule! {
3535
/// ### Invalid
3636
///
3737
/// **`foobar.js`**
38-
/// ```js
39-
/// import { baz } from "./baz.js";
38+
/// ```js,expect_diagnostic,file=foobar.js
39+
/// import { baz } from "./baz.js";
4040
///
4141
/// export function foo() {
4242
/// baz();
@@ -48,7 +48,7 @@ declare_lint_rule! {
4848
/// ```
4949
///
5050
/// **`baz.js`**
51-
/// ```js
51+
/// ```js,expect_diagnostic,file=baz.js
5252
/// import { bar } from "./foobar.js";
5353
///
5454
/// export function baz() {
@@ -59,7 +59,7 @@ declare_lint_rule! {
5959
/// ### Valid
6060
///
6161
/// **`foo.js`**
62-
/// ```js
62+
/// ```js,file=foo.js
6363
/// import { baz } from "./baz.js";
6464
///
6565
/// export function foo() {
@@ -68,14 +68,14 @@ declare_lint_rule! {
6868
/// ```
6969
///
7070
/// **`bar.js`**
71-
/// ```js
71+
/// ```js,file=bar.js
7272
/// export function bar() {
7373
/// console.log("foobar");
7474
/// }
7575
/// ```
7676
///
7777
/// **`baz.js`**
78-
/// ```js
78+
/// ```js,file=baz.js
7979
/// import { bar } from "./bar.js";
8080
///
8181
/// export function baz() {
@@ -84,7 +84,7 @@ declare_lint_rule! {
8484
/// ```
8585
///
8686
/// **`types.ts`**
87-
/// ```ts
87+
/// ```ts,file=types.ts
8888
/// import type { bar } from "./qux.ts";
8989
///
9090
/// export type Foo = {
@@ -93,7 +93,7 @@ declare_lint_rule! {
9393
/// ```
9494
///
9595
/// **`qux.ts`**
96-
/// ```ts
96+
/// ```ts,file=qux.ts
9797
/// import type { Foo } from "./types.ts";
9898
///
9999
/// export function bar(foo: Foo) {

xtask/rules_check/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,11 @@ biome_json_analyze = { workspace = true }
2626
biome_json_factory = { workspace = true }
2727
biome_json_parser = { workspace = true }
2828
biome_json_syntax = { workspace = true }
29+
biome_module_graph = { workspace = true }
30+
biome_project_layout = { workspace = true }
2931
biome_rowan = { workspace = true }
3032
biome_service = { workspace = true }
33+
biome_test_utils = { workspace = true }
3134
camino = { workspace = true }
3235
pulldown-cmark = "0.13.0"
3336

xtask/rules_check/src/lib.rs

Lines changed: 148 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,24 +12,28 @@ use biome_css_parser::CssParserOptions;
1212
use biome_css_syntax::CssLanguage;
1313
use biome_deserialize::json::deserialize_from_json_ast;
1414
use biome_diagnostics::{DiagnosticExt, PrintDiagnostic, Severity};
15-
use biome_fs::BiomePath;
15+
use biome_fs::{BiomePath, MemoryFileSystem};
1616
use biome_graphql_syntax::GraphqlLanguage;
1717
use biome_js_analyze::JsAnalyzerServices;
1818
use biome_js_parser::JsParserOptions;
1919
use biome_js_syntax::{EmbeddingKind, JsFileSource, JsLanguage, TextSize};
2020
use biome_json_factory::make;
2121
use biome_json_parser::JsonParserOptions;
2222
use biome_json_syntax::{AnyJsonValue, JsonLanguage, JsonObjectValue};
23+
use biome_module_graph::ModuleGraph;
24+
use biome_project_layout::ProjectLayout;
2325
use biome_rowan::AstNode;
2426
use biome_service::projects::{ProjectKey, Projects};
2527
use biome_service::settings::ServiceLanguage;
2628
use biome_service::workspace::DocumentFileSource;
29+
use biome_test_utils::get_added_paths;
2730
use camino::Utf8PathBuf;
28-
use pulldown_cmark::{CodeBlockKind, Event, Parser, Tag, TagEnd};
29-
use std::collections::BTreeMap;
31+
use pulldown_cmark::{CodeBlockKind, Event, HeadingLevel, Parser, Tag, TagEnd};
32+
use std::collections::{BTreeMap, HashMap};
3033
use std::fmt::{Display, Formatter, Write};
3134
use std::slice;
3235
use std::str::FromStr;
36+
use std::sync::Arc;
3337

3438
#[derive(Debug)]
3539
struct Errors(String);
@@ -169,6 +173,9 @@ struct CodeBlockTest {
169173
/// Whether to use the last code block that was marked with
170174
/// `options` as the configuration settings for this code block.
171175
use_options: bool,
176+
177+
/// The given file path in the testing in memory file system if provided.
178+
file_path: Option<String>,
172179
}
173180

174181
#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
@@ -207,9 +214,26 @@ impl FromStr for CodeBlockTest {
207214
ignore: false,
208215
options: OptionsParsingMode::NoOptions,
209216
use_options: false,
217+
file_path: None,
210218
};
211219

212220
for token in tokens {
221+
// Handle file=path attribute to create multi-file test scenarios
222+
if let Some(file) = token.strip_prefix("file=") {
223+
if file.is_empty() {
224+
bail!("The 'file' attribute must be followed by a file path");
225+
}
226+
227+
// Normalize to absolute paths for consistent module resolution
228+
let path = file
229+
.trim_start_matches("./")
230+
.trim_start_matches("../")
231+
.trim();
232+
test.file_path = Some(format!("/{}", path));
233+
234+
continue;
235+
}
236+
213237
match token {
214238
// Other attributes
215239
"expect_diagnostic" => test.expect_diagnostic = true,
@@ -348,8 +372,12 @@ fn assert_lint(
348372
test: &CodeBlockTest,
349373
code: &str,
350374
config: &Option<Configuration>,
375+
test_files: &HashMap<String, String>,
351376
) -> anyhow::Result<()> {
352-
let file_path = format!("code-block.{}", test.tag);
377+
let file_path = test
378+
.file_path
379+
.clone()
380+
.unwrap_or_else(|| format!("code-block.{}", test.tag));
353381

354382
if test.ignore {
355383
return Ok(());
@@ -419,8 +447,13 @@ fn assert_lint(
419447
test,
420448
);
421449

422-
let services =
423-
JsAnalyzerServices::from((Default::default(), Default::default(), file_source));
450+
let module_graph = get_test_module_graph(test_files);
451+
452+
let services = JsAnalyzerServices::from((
453+
Arc::new(module_graph),
454+
Default::default(),
455+
file_source,
456+
));
424457

425458
biome_js_analyze::analyze(&root, filter, &options, &[], services, |signal| {
426459
if let Some(mut diag) = signal.diagnostic() {
@@ -811,6 +844,8 @@ fn parse_documentation(
811844
) -> anyhow::Result<()> {
812845
let parser = Parser::new(rule_metadata.docs);
813846

847+
let mut test_runner = TestRunner::new(group, rule_metadata.name);
848+
814849
// Track the last configuration options block that was encountered
815850
let mut last_options: Option<Configuration> = None;
816851

@@ -831,7 +866,17 @@ fn parse_documentation(
831866
last_options =
832867
parse_rule_options(group, &rule_metadata, category, &test, &block)?;
833868
} else {
834-
assert_lint(group, rule_metadata.name, &test, &block, &last_options)?;
869+
if let Some(file_path) = &test.file_path {
870+
test_runner
871+
.file_system
872+
.insert(file_path.clone(), block.clone());
873+
}
874+
875+
test_runner.pending_tests.push(PendingTest {
876+
test,
877+
block,
878+
options_snapshot: last_options.clone(),
879+
});
835880
}
836881
}
837882
}
@@ -845,10 +890,106 @@ fn parse_documentation(
845890
}
846891
}
847892
}
893+
Event::Start(Tag::Heading { level, .. }) => {
894+
// Major headings delineate testable sections. When we encounter a new section,
895+
// run all tests from the previous section with the complete file system.
896+
if matches!(
897+
level,
898+
HeadingLevel::H1 | HeadingLevel::H2 | HeadingLevel::H3 | HeadingLevel::H4
899+
) {
900+
test_runner.run_pending_tests()?;
901+
}
902+
}
848903
// We don't care other events
849904
_ => {}
850905
}
851906
}
852907

908+
test_runner.run_pending_tests()?;
909+
853910
Ok(())
854911
}
912+
913+
struct PendingTest {
914+
/// The test definition
915+
test: CodeBlockTest,
916+
/// The code block content for the test
917+
block: String,
918+
/// The last encountered configuration options block seen before this test was collected.
919+
/// We take a copy of the options because one document may contain multiple options blocks.
920+
options_snapshot: Option<Configuration>,
921+
}
922+
923+
/// The test runner collects code block tests into batches grouped by documentation sections
924+
/// (delineated by markdown headings). It gathers all context required for each test,
925+
/// including options and in-memory files that may be referenced by the code blocks.
926+
struct TestRunner {
927+
group: &'static str,
928+
rule_name: &'static str,
929+
930+
/// Code block tests for the current documentation section.
931+
/// Tests are deferred and run as a batch when the section ends.
932+
pub pending_tests: Vec<PendingTest>,
933+
934+
/// In-memory file system for code blocks annotated with `file=path`.
935+
/// All files are collected before running tests, ensuring each test
936+
/// has access to the complete file system regardless of definition order.
937+
/// This is essential for multi-file rules like import cycle detection.
938+
pub file_system: HashMap<String, String>,
939+
}
940+
941+
impl TestRunner {
942+
pub fn new(group: &'static str, rule_name: &'static str) -> Self {
943+
Self {
944+
group,
945+
rule_name,
946+
pending_tests: Vec::new(),
947+
file_system: HashMap::new(),
948+
}
949+
}
950+
951+
/// Runs all pending tests with the current file system, then resets state for the next section.
952+
pub fn run_pending_tests(&mut self) -> anyhow::Result<()> {
953+
for test in &self.pending_tests {
954+
assert_lint(
955+
self.group,
956+
self.rule_name,
957+
&test.test,
958+
&test.block,
959+
&test.options_snapshot,
960+
&self.file_system,
961+
)?;
962+
}
963+
964+
self.pending_tests.clear();
965+
self.file_system.clear();
966+
967+
Ok(())
968+
}
969+
}
970+
971+
/// Creates an in-memory module graph for the given files.
972+
/// Returns an empty module graph if no files are provided.
973+
fn get_test_module_graph(files: &HashMap<String, String>) -> ModuleGraph {
974+
let module_graph = ModuleGraph::default();
975+
976+
if files.is_empty() {
977+
return module_graph;
978+
}
979+
980+
let fs = MemoryFileSystem::default();
981+
let layout = ProjectLayout::default();
982+
983+
let mut added_paths = Vec::with_capacity(files.len());
984+
985+
for (path, src) in files.iter() {
986+
let path_buf = Utf8PathBuf::from(path);
987+
added_paths.push(BiomePath::new(&path_buf));
988+
fs.insert(path_buf, src.as_bytes().to_vec());
989+
}
990+
991+
let added_paths = get_added_paths(&fs, &added_paths);
992+
module_graph.update_graph_for_js_paths(&fs, &layout, &added_paths, &[]);
993+
994+
module_graph
995+
}

0 commit comments

Comments
 (0)