Skip to content

Commit

Permalink
fix(cli): inject source code in diagnostics printed in stdin (#3211)
Browse files Browse the repository at this point in the history
Co-authored-by: Arend van Beelen jr. <[email protected]>
  • Loading branch information
ematipico and arendjr authored Jun 14, 2024
1 parent 743d805 commit 219dae0
Show file tree
Hide file tree
Showing 15 changed files with 314 additions and 31 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

#### Bug fixes

- Fix [#3201](https://github.com/biomejs/biome/issues/3201) by correctly injecting the source code of the file when printing the diagnostics. Contributed by @ematipico
- Fix [#3179](https://github.com/biomejs/biome/issues/3179) where comma separators are not correctly removed after running `biome migrate` and thus choke the parser. Contributed by @Sec-ant

#### Enhancement
Expand Down
23 changes: 18 additions & 5 deletions crates/biome_cli/src/execute/std_in.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use crate::execute::diagnostics::{ContentDiffAdvice, FormatDiffDiagnostic};
use crate::execute::Execution;
use crate::{CliDiagnostic, CliSession, TraversalMode};
use biome_console::{markup, ConsoleExt};
use biome_diagnostics::Diagnostic;
use biome_diagnostics::PrintDiagnostic;
use biome_diagnostics::{Diagnostic, DiagnosticExt, Error};
use biome_fs::BiomePath;
use biome_service::file_handlers::{AstroFileHandler, SvelteFileHandler, VueFileHandler};
use biome_service::workspace::{
Expand Down Expand Up @@ -75,7 +75,7 @@ pub(crate) fn run<'a>(
})
}
} else if mode.is_check() || mode.is_lint() {
let mut diagnostics = Vec::new();
let mut diagnostics: Vec<Error> = Vec::new();
let mut new_content = Cow::Borrowed(content);

workspace.open_file(OpenFileParams {
Expand Down Expand Up @@ -169,7 +169,20 @@ pub(crate) fn run<'a>(
only,
skip,
})?;
diagnostics.extend(result.diagnostics);
let content = match biome_path.extension_as_str() {
Some("astro") => AstroFileHandler::input(&new_content),
Some("vue") => VueFileHandler::input(&new_content),
Some("svelte") => SvelteFileHandler::input(&new_content),
_ => &new_content,
};
diagnostics.extend(
result
.diagnostics
.into_iter()
.map(Error::from)
.map(|diag| diag.with_file_source_code(content.to_string()))
.collect::<Vec<_>>(),
);
}

if file_features.supports_format() && mode.is_check() {
Expand All @@ -195,7 +208,7 @@ pub(crate) fn run<'a>(
old: content.to_string(),
},
};
diagnostics.push(biome_diagnostics::serde::Diagnostic::new(diagnostic));
diagnostics.push(Error::from(diagnostic));
}
}

Expand All @@ -205,7 +218,7 @@ pub(crate) fn run<'a>(
{original_content}
});
}
Cow::Owned(new_content) => {
Cow::Owned(ref new_content) => {
console.append(markup! {
{new_content}
});
Expand Down
32 changes: 32 additions & 0 deletions crates/biome_cli/tests/commands/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2276,6 +2276,38 @@ fn check_stdin_write_successfully() {
));
}

#[test]
fn check_stdin_shows_parse_diagnostics() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

console.in_buffer.push(
r#"
server.get('/', async () => {
return { hello: 'world' };
});
const = ""; "#
.to_string(),
);

let result = run_cli(
DynRef::Borrowed(&mut fs),
&mut console,
Args::from([("lint"), "--write", ("--stdin-file-path"), ("mock.ts")].as_slice()),
);

assert!(result.is_ok(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"check_stdin_shows_parse_diagnostics",
fs,
console,
result,
));
}

#[test]
fn check_stdin_write_unsafe_successfully() {
let mut fs = MemoryFileSystem::default();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,15 @@ var foo: string = "";
```

```block
file.astro lint/complexity/noUselessRename FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.astro:2:9 lint/complexity/noUselessRename FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Useless rename.
> 2 │ import {a as a} from 'mod';
│ ^^^^^^
3 │ import { something } from "file.astro";
4 │ debugger;
i Safe fix: Remove the renaming.
2 │ import·{a·as·a}·from·'mod';
Expand All @@ -41,10 +46,17 @@ file.astro lint/complexity/noUselessRename FIXABLE ━━━━━━━━━
```

```block
file.astro lint/suspicious/noDebugger FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.astro:4:1 lint/suspicious/noDebugger FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× This is an unexpected use of the debugger statement.
2 │ import {a as a} from 'mod';
3 │ import { something } from "file.astro";
> 4 │ debugger;
│ ^^^^^^^^^
5 │ statement ( ) ;
6 │ var foo: string = "";
i Unsafe fix: Remove debugger statement
2 2 │ import {a as a} from 'mod';
Expand All @@ -57,10 +69,16 @@ file.astro lint/suspicious/noDebugger FIXABLE ━━━━━━━━━━
```

```block
file.astro lint/style/noInferrableTypes FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.astro:6:8 lint/style/noInferrableTypes FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× This type annotation is trivially inferred from its initialization.
4 │ debugger;
5 │ statement ( ) ;
> 6 │ var foo: string = "";
│ ^^^^^^^^
7 │
i Safe fix: Remove the type annotation.
4 4 │ debugger;
Expand All @@ -73,10 +91,16 @@ file.astro lint/style/noInferrableTypes FIXABLE ━━━━━━━━━━
```

```block
file.astro lint/style/noVar FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.astro:6:1 lint/style/noVar FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Use let or const instead of var.
4 │ debugger;
5 │ statement ( ) ;
> 6 │ var foo: string = "";
│ ^^^^^^^^^^^^^^^^^^^^
7 │
i A variable declared with var is accessible in the whole module. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
i See MDN web docs for more details.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,17 @@ var foo = "";
```

```block
file.astro lint/suspicious/noDebugger FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.astro:4:1 lint/suspicious/noDebugger FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× This is an unexpected use of the debugger statement.
2 │ import { something } from "file.astro";
3 │ import { a } from "mod";
> 4 │ debugger;
│ ^^^^^^^^^
5 │ statement();
6 │ var foo = "";
i Unsafe fix: Remove debugger statement
2 2 │ import { something } from "file.astro";
Expand All @@ -45,10 +52,16 @@ file.astro lint/suspicious/noDebugger FIXABLE ━━━━━━━━━━
```

```block
file.astro lint/style/noVar FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.astro:6:1 lint/style/noVar FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Use let or const instead of var.
4 │ debugger;
5 │ statement();
> 6 │ var foo = "";
│ ^^^^^^^^^^^^
7 │
i A variable declared with var is accessible in the whole module. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
i See MDN web docs for more details.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,14 @@ import {a as a} from 'mod';
```

```block
file.astro lint/complexity/noUselessRename FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.astro:2:9 lint/complexity/noUselessRename FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Useless rename.
> 2 │ import {a as a} from 'mod';
│ ^^^^^^
3 │
i Safe fix: Remove the renaming.
2 │ import·{a·as·a}·from·'mod';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,15 @@ var foo: string = "";
```

```block
file.svelte lint/complexity/noUselessRename FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.svelte:1:10 lint/complexity/noUselessRename FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Useless rename.
> 1 │ import { Form as Form } from './components/Form.svelte' ;
│ ^^^^^^^^^^^^^^
2 │ import Button from "./components/Button.svelte";
3 │ debugger;
i Safe fix: Remove the renaming.
1 │ import·{·Form·as···Form·}·····from·'./components/Form.svelte'·;
Expand All @@ -41,10 +46,17 @@ file.svelte lint/complexity/noUselessRename FIXABLE ━━━━━━━━
```

```block
file.svelte lint/suspicious/noDebugger FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.svelte:3:1 lint/suspicious/noDebugger FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× This is an unexpected use of the debugger statement.
1 │ import { Form as Form } from './components/Form.svelte' ;
2 │ import Button from "./components/Button.svelte";
> 3 │ debugger;
│ ^^^^^^^^^
4 │ statement ( ) ;
5 │ var foo: string = "";
i Unsafe fix: Remove debugger statement
1 1 │ import { Form as Form } from './components/Form.svelte' ;
Expand All @@ -57,10 +69,16 @@ file.svelte lint/suspicious/noDebugger FIXABLE ━━━━━━━━━━
```

```block
file.svelte lint/style/noInferrableTypes FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.svelte:5:8 lint/style/noInferrableTypes FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× This type annotation is trivially inferred from its initialization.
3 │ debugger;
4 │ statement ( ) ;
> 5 │ var foo: string = "";
│ ^^^^^^^^
6 │
i Safe fix: Remove the type annotation.
3 3 │ debugger;
Expand All @@ -73,10 +91,16 @@ file.svelte lint/style/noInferrableTypes FIXABLE ━━━━━━━━━
```

```block
file.svelte lint/style/noVar FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.svelte:5:1 lint/style/noVar FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Use let or const instead of var.
3 │ debugger;
4 │ statement ( ) ;
> 5 │ var foo: string = "";
│ ^^^^^^^^^^^^^^^^^^^^
6 │
i A variable declared with var is accessible in the whole module. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
i See MDN web docs for more details.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,17 @@ var foo = "";
```

```block
file.svelte lint/suspicious/noDebugger FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.svelte:3:1 lint/suspicious/noDebugger FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× This is an unexpected use of the debugger statement.
1 │ import Button from "./components/Button.svelte";
2 │ import { Form } from "./components/Form.svelte";
> 3 │ debugger;
│ ^^^^^^^^^
4 │ statement();
5 │ var foo = "";
i Unsafe fix: Remove debugger statement
1 1 │ import Button from "./components/Button.svelte";
Expand All @@ -45,10 +52,16 @@ file.svelte lint/suspicious/noDebugger FIXABLE ━━━━━━━━━━
```

```block
file.svelte lint/style/noVar FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.svelte:5:1 lint/style/noVar FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Use let or const instead of var.
3 │ debugger;
4 │ statement();
> 5 │ var foo = "";
│ ^^^^^^^^^^^^
6 │
i A variable declared with var is accessible in the whole module. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
i See MDN web docs for more details.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,14 @@ var foo: string = "";
```

```block
file.svelte lint/style/noInferrableTypes FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.svelte:1:8 lint/style/noInferrableTypes FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× This type annotation is trivially inferred from its initialization.
> 1 │ var foo: string = "";
│ ^^^^^^^^
2 │
i Safe fix: Remove the type annotation.
1 │ - var·foo:·string·=·"";
Expand All @@ -35,10 +39,14 @@ file.svelte lint/style/noInferrableTypes FIXABLE ━━━━━━━━━
```

```block
file.svelte lint/style/noVar FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.svelte:1:1 lint/style/noVar FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Use let or const instead of var.
> 1 │ var foo: string = "";
│ ^^^^^^^^^^^^^^^^^^^^
2 │
i A variable declared with var is accessible in the whole module. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
i See MDN web docs for more details.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,14 @@ var foo = "";
```

```block
file.svelte lint/style/noVar FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.svelte:1:1 lint/style/noVar FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Use let or const instead of var.
> 1 │ var foo = "";
│ ^^^^^^^^^^^^
2 │
i A variable declared with var is accessible in the whole module. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
i See MDN web docs for more details.
Expand Down
Loading

0 comments on commit 219dae0

Please sign in to comment.