Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_lsp): improve the pattern matching logic for ignored files #4024

Merged
merged 5 commits into from
Dec 13, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
56 changes: 34 additions & 22 deletions crates/rome_cli/tests/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1104,17 +1104,29 @@ fn does_not_format_if_files_are_listed_in_ignore_option() {
fn does_not_format_ignored_directories() {
let mut console = BufferConsole::default();
let mut fs = MemoryFileSystem::default();

let file_path = Path::new("rome.json");
fs.insert(
file_path.into(),
CONFIG_FORMATTER_IGNORED_DIRECTORIES.as_bytes(),
);

let ignored_file = Path::new("scripts/test.js");
fs.insert(ignored_file.into(), <&str>::clone(&UNFORMATTED).as_bytes());

let file_to_format = Path::new("src/test.js");
fs.insert(file_to_format.into(), UNFORMATTED.as_bytes());
const FILES: [(&str, bool); 9] = [
("test.js", true),
("test1.js", false),
("test2.js", false),
("test3/test.js", false),
("test4/test.js", true),
("test5/test.js", false),
("test6/test.js", false),
("test/test.test7.js", false),
("test.test7.js", false),
];

for (file_path, _) in FILES {
let file_path = Path::new(file_path);
fs.insert(file_path.into(), UNFORMATTED.as_bytes());
}

let result = run_cli(
DynRef::Borrowed(&mut fs),
Expand All @@ -1128,26 +1140,26 @@ fn does_not_format_ignored_directories() {

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

let mut file = fs
.open(ignored_file)
.expect("formatting target file was removed by the CLI");
for (file_path, expect_formatted) in FILES {
let mut file = fs
.open(Path::new(file_path))
.expect("formatting target file was removed by the CLI");

let mut content = String::new();
file.read_to_string(&mut content)
.expect("failed to read file from memory FS");

assert_eq!(content, UNFORMATTED, "we test the file is not formatted");
drop(file);
let mut file = fs
.open(file_to_format)
.expect("formatting target file was removed by the CLI");
let mut content = String::new();
file.read_to_string(&mut content)
.expect("failed to read file from memory FS");

let mut content = String::new();
file.read_to_string(&mut content)
.expect("failed to read file from memory FS");
let expected = if expect_formatted {
FORMATTED
} else {
UNFORMATTED
};

assert_eq!(content, FORMATTED, "we test the file is formatted");
drop(file);
assert_eq!(
content, expected,
"content of {file_path} doesn't match the expected content"
);
}

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
Expand Down
10 changes: 9 additions & 1 deletion crates/rome_cli/tests/configs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,15 @@ pub const CONFIG_FORMATTER_AND_FILES_IGNORE: &str = r#"{

pub const CONFIG_FORMATTER_IGNORED_DIRECTORIES: &str = r#"{
"formatter": {
"ignore": ["scripts/*"]
"ignore": [
"test1.js",
"./test2.js",
"./test3/**/*",
"/test4/**/*",
"test5/**/*",
"**/test6/*.js",
"*.test7.js"
]
}
}
"#;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,80 @@ expression: content
```json
{
"formatter": {
"ignore": ["scripts/*"]
"ignore": [
"test1.js",
"./test2.js",
"./test3/**/*",
"/test4/**/*",
"test5/**/*",
"**/test6/*.js",
"*.test7.js"
]
}
}

```

## `scripts/test.js`
## `test.js`

```js
statement();

```

## `test.test7.js`

```js
statement( )
```

## `test/test.test7.js`

```js
statement( )
```

## `test1.js`

```js
statement( )
```

## `test2.js`

```js
statement( )
```

## `src/test.js`
## `test3/test.js`

```js
statement( )
```

## `test4/test.js`

```js
statement();

```

## `test5/test.js`

```js
statement( )
```

## `test6/test.js`

```js
statement( )
```

# Emitted Messages

```block
Formatted 2 file(s) in <TIME>
Formatted 3 file(s) in <TIME>
```


4 changes: 2 additions & 2 deletions crates/rome_lsp/src/handlers/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub(crate) fn code_actions(
params: CodeActionParams,
) -> Result<Option<CodeActionResponse>> {
let url = params.text_document.uri.clone();
let rome_path = session.file_path(&url);
let rome_path = session.file_path(&url)?;

let unsupported_lint = &session.workspace.supports_feature(SupportsFeatureParams {
path: rome_path,
Expand All @@ -57,7 +57,7 @@ pub(crate) fn code_actions(
}

let url = params.text_document.uri.clone();
let rome_path = session.file_path(&url);
let rome_path = session.file_path(&url)?;
let doc = session.document(&url)?;

let diagnostics = params.context.diagnostics;
Expand Down
6 changes: 3 additions & 3 deletions crates/rome_lsp/src/handlers/formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub(crate) fn format(
params: DocumentFormattingParams,
) -> Result<Option<Vec<TextEdit>>> {
let url = params.text_document.uri;
let rome_path = session.file_path(&url);
let rome_path = session.file_path(&url)?;

let doc = session.document(&url)?;

Expand Down Expand Up @@ -56,7 +56,7 @@ pub(crate) fn format_range(
params: DocumentRangeFormattingParams,
) -> Result<Option<Vec<TextEdit>>> {
let url = params.text_document.uri;
let rome_path = session.file_path(&url);
let rome_path = session.file_path(&url)?;
let doc = session.document(&url)?;

let start_index = utils::offset(&doc.line_index, params.range.start).with_context(|| {
Expand Down Expand Up @@ -119,7 +119,7 @@ pub(crate) fn format_on_type(
let url = params.text_document_position.text_document.uri;
let position = params.text_document_position.position;

let rome_path = session.file_path(&url);
let rome_path = session.file_path(&url)?;
let doc = session.document(&url)?;

let offset = utils::offset(&doc.line_index, position)
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_lsp/src/handlers/rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use tracing::trace;
#[tracing::instrument(level = "debug", skip(session), err)]
pub(crate) fn rename(session: &Session, params: RenameParams) -> Result<Option<WorkspaceEdit>> {
let url = params.text_document_position.text_document.uri;
let rome_path = session.file_path(&url);
let rome_path = session.file_path(&url)?;

trace!("Renaming...");

Expand Down
6 changes: 3 additions & 3 deletions crates/rome_lsp/src/handlers/text_document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub(crate) async fn did_open(
let content = params.text_document.text;
let language_hint = Language::from_language_id(&params.text_document.language_id);

let rome_path = session.file_path(&url);
let rome_path = session.file_path(&url)?;
let doc = Document::new(version, &content);

session.workspace.open_file(OpenFileParams {
Expand Down Expand Up @@ -46,7 +46,7 @@ pub(crate) async fn did_change(
let url = params.text_document.uri;
let version = params.text_document.version;

let rome_path = session.file_path(&url);
let rome_path = session.file_path(&url)?;
let doc = session.document(&url)?;

let mut content = doc.content;
Expand Down Expand Up @@ -93,7 +93,7 @@ pub(crate) async fn did_close(
params: lsp_types::DidCloseTextDocumentParams,
) -> Result<()> {
let url = params.text_document.uri;
let rome_path = session.file_path(&url);
let rome_path = session.file_path(&url)?;

session
.workspace
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_lsp/src/requests/syntax_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub struct SyntaxTreePayload {

pub(crate) fn syntax_tree(session: &Session, url: &Url) -> Result<String> {
info!("Showing syntax tree");
let rome_path = session.file_path(url);
let rome_path = session.file_path(url)?;
let syntax_tree = session
.workspace
.get_syntax_tree(GetSyntaxTreeParams { path: rome_path })?;
Expand Down
25 changes: 21 additions & 4 deletions crates/rome_lsp/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::config::CONFIGURATION_SECTION;
use crate::documents::Document;
use crate::url_interner::UrlInterner;
use crate::utils;
use anyhow::{anyhow, Result};
use futures::stream::futures_unordered::FuturesUnordered;
use futures::StreamExt;
use rome_analyze::RuleCategories;
Expand Down Expand Up @@ -127,17 +128,33 @@ impl Session {
self.url_interner.write().unwrap().intern(url)
}

pub(crate) fn file_path(&self, url: &lsp_types::Url) -> RomePath {
pub(crate) fn file_path(&self, url: &lsp_types::Url) -> Result<RomePath> {
let file_id = self.file_id(url.clone());
RomePath::new(url.path(), file_id)
let mut path_to_file = url
.to_file_path()
.map_err(|()| anyhow!("failed to convert {url} to a filesystem path"))?;
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this macro do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The anyhow! macro works like format! except it returns an instance of anyhow::Error instead of a String


let relative_path = {
let root_uri = self.root_uri.read().unwrap();
root_uri.as_ref().and_then(|root_uri| {
let root_path = root_uri.to_file_path().ok()?;
path_to_file.strip_prefix(&root_path).ok()
})
};

if let Some(relative_path) = relative_path {
path_to_file = relative_path.into();
}

Ok(RomePath::new(path_to_file, file_id))
}

/// Computes diagnostics for the file matching the provided url and publishes
/// them to the client. Called from [`handlers::text_document`] when a file's
/// contents changes.
#[tracing::instrument(level = "debug", skip_all, fields(url = display(&url), diagnostic_count), err)]
pub(crate) async fn update_diagnostics(&self, url: lsp_types::Url) -> anyhow::Result<()> {
let rome_path = self.file_path(&url);
pub(crate) async fn update_diagnostics(&self, url: lsp_types::Url) -> Result<()> {
let rome_path = self.file_path(&url)?;
let doc = self.document(&url)?;
let unsupported_lint = self.workspace.supports_feature(SupportsFeatureParams {
feature: FeatureName::Lint,
Expand Down
Loading