Skip to content

Commit

Permalink
refactor(cli): do not emit warnings for protected ignored files (biom…
Browse files Browse the repository at this point in the history
  • Loading branch information
Conaclos authored and ematipico committed Jan 24, 2024
1 parent de9b3e1 commit 801c9ba
Show file tree
Hide file tree
Showing 7 changed files with 319 additions and 78 deletions.
17 changes: 16 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,26 @@ Read our [guidelines for writing a good changelog entry](https:/biom

### CLI

### Bug fixes

- Fix [#1512](https:/biomejs/biome/issues/1512) by skipping verbose diagnostics from the count. Contributed by @ematipico
- Don't handle CSS files, the formatter isn't ready yet. Contributed by @ematipico

- Don't emit verbose warnings when a protected file is ignored.

Some files, such as `package.json` and `tsconfig.json`, are [protected](https://biomejs.dev/guides/how-biome-works/#protected-files).
Biome emits a verbose warning when it encounters a protected file.

Previously, Biome emitted this verbose warning even if the file was ignored by the configuration.
Now, it doesn't emit verbose warnings for protected files that are ignored.

Contributed by @Conaclos

- The file `biome.json` can't be ignored anymore. Contributed by @ematipico

- Fix [#1541](https:/biomejs/biome/issues/1541) where the content of protected files wasn't returned to `stdout`. Contributed by @ematipico

- Don't handle CSS files, the formatter isn't ready yet. Contributed by @ematipico

### Configuration

#### Bug fixes
Expand Down
83 changes: 83 additions & 0 deletions crates/biome_cli/tests/cases/protected_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ fn not_process_file_from_cli() {
result,
));
}

#[test]
fn not_process_file_from_cli_verbose() {
let mut fs = MemoryFileSystem::default();
Expand Down Expand Up @@ -158,6 +159,88 @@ fn not_process_file_from_cli_verbose() {
));
}

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

let file_path = Path::new("package.json");
fs.insert(file_path.into(), r#"{ "name": "test" }"#.as_bytes());

let file_path = Path::new("other.json");
fs.insert(file_path.into(), r#"{}"#.as_bytes());

let file_path1 = Path::new("biome.json");
fs.insert(
file_path1.into(),
r#"{ "files": { "ignore": ["package.json"] } }"#.as_bytes(),
);

let result = run_cli(
DynRef::Borrowed(&mut fs),
&mut console,
Args::from(
[
("format"),
"--verbose",
file_path.as_os_str().to_str().unwrap(),
]
.as_slice(),
),
);

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

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

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

let file_path = Path::new("package.json");
fs.insert(file_path.into(), r#"{ "name": "test" }"#.as_bytes());

let file_path = Path::new("other.json");
fs.insert(file_path.into(), r#"{}"#.as_bytes());

let file_path1 = Path::new("biome.json");
fs.insert(
file_path1.into(),
r#"{ "linter": { "enabled": false } }"#.as_bytes(),
);

let result = run_cli(
DynRef::Borrowed(&mut fs),
&mut console,
Args::from(
[
("format"),
"--verbose",
file_path.as_os_str().to_str().unwrap(),
]
.as_slice(),
),
);

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

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

#[test]
fn should_return_the_content_of_protected_files_via_stdin() {
let mut fs = MemoryFileSystem::default();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: content
---
## `biome.json`

```json
{ "linter": { "enabled": false } }
```

## `other.json`

```json
{}
```

## `package.json`

```json
{ "name": "test" }
```

# Termination Message

```block
format ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Some errors were emitted while running checks.
```

# Emitted Messages

```block
other.json format ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
i Formatter would have printed the following content:
1 │ - {}
1 │ + {}
2 │ +
```

```block
Compared 1 file(s) in <TIME>
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: content
---
## `biome.json`

```json
{ "files": { "ignore": ["package.json"] } }
```

## `other.json`

```json
{}
```

## `package.json`

```json
{ "name": "test" }
```

# Termination Message

```block
format ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Some errors were emitted while running checks.
```

# Emitted Messages

```block
other.json format ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
i Formatter would have printed the following content:
1 │ - {}
1 │ + {}
2 │ +
```

```block
Compared 1 file(s) in <TIME>
```


126 changes: 76 additions & 50 deletions crates/biome_service/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,29 +95,38 @@ pub struct FileFeaturesResult {
}

impl FileFeaturesResult {
/// Files that should not be processed no matter the cases
pub(crate) const FILES_TO_NOT_PROCESS: &'static [&'static str; 12] = &[
"package.json",
"package-lock.json",
"npm-shrinkwrap.json",
"yarn.lock",
/// Sorted array of files that should not be processed no matter the cases.
/// These files are handled by other tools.
const PROTECTED_FILES: &'static [&'static str; 11] = &[
// Composer
"composer.json",
"composer.lock",
"typescript.json",
"tsconfig.json",
"jsconfig.json",
// Deno
"deno.json",
"deno.jsonc",
// TODO: remove this when are able to handle nested .gitignore files
".gitignore",
// TSC
"jsconfig.json",
// NPM
"npm-shrinkwrap.json",
"package-lock.json",
"package.json",
// TSC
"tsconfig.json",
"typescript.json",
// Yarn
"yarn.lock",
];

/// Checks whether this file can be processed
fn can_process(&self, path: &Path) -> bool {
/// Checks whether this file is protected.
/// A protected file is handled by a specific tool and should be ignored.
pub(crate) fn is_protected_file(path: &Path) -> bool {
path.file_name()
.and_then(OsStr::to_str)
.map(|file_name| !FileFeaturesResult::FILES_TO_NOT_PROCESS.contains(&file_name))
.unwrap_or_default()
.is_some_and(|file_name| {
FileFeaturesResult::PROTECTED_FILES
.binary_search(&file_name)
.is_ok()
})
}

/// By default, all features are not supported by a file.
Expand Down Expand Up @@ -156,48 +165,44 @@ impl FileFeaturesResult {
language: &Language,
path: &Path,
) -> Self {
if self.can_process(path) {
let formatter_disabled =
if let Some(disabled) = settings.override_settings.formatter_disabled(path) {
disabled
} else if language.is_javascript_like() {
!settings.formatter().enabled || settings.javascript_formatter_disabled()
} else if language.is_json_like() {
!settings.formatter().enabled || settings.json_formatter_disabled()
} else if language.is_css_like() {
!can_format_css_yet()
|| !settings.formatter().enabled
|| settings.css_formatter_disabled()
} else {
!settings.formatter().enabled
};
if formatter_disabled {
self.features_supported
.insert(FeatureName::Format, SupportKind::FeatureNotEnabled);
}
// linter
if let Some(disabled) = settings.override_settings.linter_disabled(path) {
if disabled {
self.features_supported
.insert(FeatureName::Lint, SupportKind::FeatureNotEnabled);
}
} else if !settings.linter().enabled {
let formatter_disabled =
if let Some(disabled) = settings.override_settings.formatter_disabled(path) {
disabled
} else if language.is_javascript_like() {
!settings.formatter().enabled || settings.javascript_formatter_disabled()
} else if language.is_json_like() {
!settings.formatter().enabled || settings.json_formatter_disabled()
} else if language.is_css_like() {
!can_format_css_yet()
|| !settings.formatter().enabled
|| settings.css_formatter_disabled()
} else {
!settings.formatter().enabled
};
if formatter_disabled {
self.features_supported
.insert(FeatureName::Format, SupportKind::FeatureNotEnabled);
}
// linter
if let Some(disabled) = settings.override_settings.linter_disabled(path) {
if disabled {
self.features_supported
.insert(FeatureName::Lint, SupportKind::FeatureNotEnabled);
}
} else if !settings.linter().enabled {
self.features_supported
.insert(FeatureName::Lint, SupportKind::FeatureNotEnabled);
}

// organize imports
if let Some(disabled) = settings.override_settings.organize_imports_disabled(path) {
if disabled {
self.features_supported
.insert(FeatureName::OrganizeImports, SupportKind::FeatureNotEnabled);
}
} else if !settings.organize_imports().enabled {
// organize imports
if let Some(disabled) = settings.override_settings.organize_imports_disabled(path) {
if disabled {
self.features_supported
.insert(FeatureName::OrganizeImports, SupportKind::FeatureNotEnabled);
}
} else {
self.features_supported = HashMap::from(FileFeaturesResult::WORKSPACE_FEATURES);
} else if !settings.organize_imports().enabled {
self.features_supported
.insert(FeatureName::OrganizeImports, SupportKind::FeatureNotEnabled);
}

debug!(
Expand Down Expand Up @@ -283,6 +288,20 @@ impl FileFeaturesResult {
.values()
.all(|support_kind| support_kind.is_not_enabled())
}

/// The file is not processed if for every enabled feature
/// the file is either protected, not supported, ignored.
pub fn is_not_processed(&self) -> bool {
self.features_supported.values().all(|support_kind| {
matches!(
support_kind,
SupportKind::FeatureNotEnabled
| SupportKind::FileNotSupported
| SupportKind::Ignored
| SupportKind::Protected
)
})
}
}

impl SupportsFeatureResult {
Expand Down Expand Up @@ -859,3 +878,10 @@ impl<'app, W: Workspace + ?Sized> Drop for FileGuard<'app, W> {
.ok();
}
}

#[test]
fn test_order() {
for items in FileFeaturesResult::PROTECTED_FILES.windows(2) {
assert!(items[0] < items[1], "{} < {}", items[0], items[1]);
}
}
Loading

0 comments on commit 801c9ba

Please sign in to comment.