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

Commit

Permalink
feat(rome_lsp): pull diagnostics for rome.json (#4296)
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico authored Mar 14, 2023
1 parent 2697226 commit af991f4
Show file tree
Hide file tree
Showing 10 changed files with 138 additions and 18 deletions.
4 changes: 2 additions & 2 deletions crates/rome_analyze/src/rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
};
use rome_console::fmt::Display;
use rome_console::{markup, MarkupBuf};
use rome_deserialize::json::{deserialize_from_json, JsonDeserialize, VisitJsonNode};
use rome_deserialize::json::{deserialize_from_json_str, JsonDeserialize, VisitJsonNode};
use rome_deserialize::Deserialized;
use rome_diagnostics::advice::CodeSuggestionAdvice;
use rome_diagnostics::location::AsSpan;
Expand Down Expand Up @@ -244,7 +244,7 @@ impl_group_language!(

pub trait DeserializableRuleOptions: Default + Sized + JsonDeserialize + VisitJsonNode {
fn from(value: String) -> Deserialized<Self> {
deserialize_from_json(&value)
deserialize_from_json_str(&value)
}
}

Expand Down
23 changes: 19 additions & 4 deletions crates/rome_deserialize/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,8 @@ pub fn with_only_known_variants(
})
}

/// It attempts to parse and deserialize a source file in JSON.
/// It attempts to parse and deserialize a source file in JSON. Diagnostics from the parse phase
/// are consumed and joined with the diagnostics emitted during the deserialization.
///
/// The data structure that needs to be deserialized needs to implement three important traits:
/// - [Default], to create a first instance of the data structure;
Expand All @@ -420,7 +421,7 @@ pub fn with_only_known_variants(
///
/// ```
/// use rome_deserialize::{DeserializationDiagnostic, VisitNode, Deserialized};
/// use rome_deserialize::json::deserialize_from_json;
/// use rome_deserialize::json::deserialize_from_json_str;
/// use rome_deserialize::json::{with_only_known_variants, has_only_known_keys, JsonDeserialize, VisitJsonNode};
/// use rome_json_syntax::{JsonLanguage, JsonSyntaxNode};
/// use rome_json_syntax::JsonRoot;
Expand Down Expand Up @@ -476,15 +477,15 @@ pub fn with_only_known_variants(
///
/// # fn main() -> Result<(), DeserializationDiagnostic> {
/// let source = r#"{ "lorem": true }"#;
/// let deserialized = deserialize_from_json::<NewConfiguration>(&source);
/// let deserialized = deserialize_from_json_str::<NewConfiguration>(&source);
/// assert!(!deserialized.has_errors());
/// assert_eq!(deserialized.into_deserialized(), NewConfiguration { lorem: true });
/// # Ok(())
/// # }
///
///
/// ```
pub fn deserialize_from_json<Output>(source: &str) -> Deserialized<Output>
pub fn deserialize_from_json_str<Output>(source: &str) -> Deserialized<Output>
where
Output: Default + VisitJsonNode + JsonDeserialize,
{
Expand All @@ -508,3 +509,17 @@ where
deserialized: output,
}
}

/// Attempts to deserialize a JSON AST, given the `Output`.
pub fn deserialize_from_json_ast<Output>(parse: JsonRoot) -> Deserialized<Output>
where
Output: Default + VisitJsonNode + JsonDeserialize,
{
let mut output = Output::default();
let mut diagnostics = vec![];
Output::deserialize_from_ast(parse, &mut output, &mut diagnostics);
Deserialized {
diagnostics: diagnostics.into_iter().map(Error::from).collect::<Vec<_>>(),
deserialized: output,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rome_analyze::{
};
use rome_console::markup;
use rome_deserialize::json::{
deserialize_from_json, has_only_known_keys, JsonDeserialize, VisitJsonNode,
deserialize_from_json_str, has_only_known_keys, JsonDeserialize, VisitJsonNode,
};
use rome_deserialize::{DeserializationDiagnostic, Deserialized, VisitNode};
use rome_js_semantic::{Capture, SemanticModel};
Expand Down Expand Up @@ -253,7 +253,7 @@ impl JsonDeserialize for HooksOptions {

impl DeserializableRuleOptions for HooksOptions {
fn from(value: String) -> Deserialized<Self> {
deserialize_from_json(&value)
deserialize_from_json_str(&value)
}
}

Expand Down
88 changes: 88 additions & 0 deletions crates/rome_lsp/tests/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,27 @@ impl Server {
.await
}

/// Opens a document with given contents and given name. The name must contain the extension too
async fn open_named_document(
&mut self,
text: impl Display,
document_name: Url,
language: impl Display,
) -> Result<()> {
self.notify(
"textDocument/didOpen",
DidOpenTextDocumentParams {
text_document: TextDocumentItem {
uri: document_name,
language_id: language.to_string(),
version: 0,
text: text.to_string(),
},
},
)
.await
}

async fn change_document(
&mut self,
version: i32,
Expand Down Expand Up @@ -754,6 +775,73 @@ async fn pull_quick_fixes() -> Result<()> {
Ok(())
}

#[tokio::test]
async fn pull_diagnostics_for_rome_json() -> Result<()> {
let factory = ServerFactory::default();
let (service, client) = factory.create().into_inner();
let (stream, sink) = client.split();
let mut server = Server::new(service);

let (sender, mut receiver) = channel(CHANNEL_BUFFER_SIZE);
let reader = tokio::spawn(client_handler(stream, sink, sender));

server.initialize().await?;
server.initialized().await?;

let incorrect_config = r#"{
"formatter": {
"indentStyle": "magic"
}
}"#;
server
.open_named_document(incorrect_config, url!("rome.json"), "json")
.await?;

let notification = tokio::select! {
msg = receiver.next() => msg,
_ = sleep(Duration::from_secs(1)) => {
panic!("timed out waiting for the server to send diagnostics")
}
};

assert_eq!(
notification,
Some(ServerNotification::PublishDiagnostics(
PublishDiagnosticsParams {
uri: url!("rome.json"),
version: Some(0),
diagnostics: vec![lsp::Diagnostic {
range: lsp::Range {
start: lsp::Position {
line: 2,
character: 27,
},
end: lsp::Position {
line: 2,
character: 34,
},
},
severity: Some(lsp::DiagnosticSeverity::ERROR),
code: Some(lsp::NumberOrString::String(String::from("configuration",))),
code_description: None,
source: Some(String::from("rome")),
message: String::from("Found an unknown value `magic`",),
related_information: None,
tags: None,
data: None,
}],
}
))
);

server.close_document().await?;

server.shutdown().await?;
reader.abort();

Ok(())
}

#[tokio::test]
async fn pull_refactors() -> Result<()> {
let factory = ServerFactory::default();
Expand Down
6 changes: 3 additions & 3 deletions crates/rome_service/src/configuration/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ pub struct InvalidIgnorePattern {
mod test {
use crate::configuration::diagnostics::ConfigurationDiagnostic;
use crate::{Configuration, MatchOptions, Matcher};
use rome_deserialize::json::deserialize_from_json;
use rome_deserialize::json::deserialize_from_json_str;
use rome_diagnostics::{print_diagnostic_to_string, DiagnosticExt, Error};

fn snap_diagnostic(test_name: &str, diagnostic: Error) {
Expand Down Expand Up @@ -268,7 +268,7 @@ mod test {
#[test]
fn deserialization_error() {
let content = "{ \n\n\"formatter\" }";
let result = deserialize_from_json::<Configuration>(content);
let result = deserialize_from_json_str::<Configuration>(content);

assert!(result.has_errors());
for diagnostic in result.into_diagnostics() {
Expand All @@ -291,6 +291,6 @@ mod test {
}
}
}"#;
let _result = deserialize_from_json::<Configuration>(content).into_deserialized();
let _result = deserialize_from_json_str::<Configuration>(content).into_deserialized();
}
}
4 changes: 2 additions & 2 deletions crates/rome_service/src/configuration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub use formatter::{FormatterConfiguration, PlainIndentStyle};
pub use javascript::{JavascriptConfiguration, JavascriptFormatter};
pub use linter::{LinterConfiguration, RuleConfiguration, Rules};
use rome_analyze::{AnalyzerConfiguration, AnalyzerRules};
use rome_deserialize::json::deserialize_from_json;
use rome_deserialize::json::deserialize_from_json_str;
use rome_deserialize::Deserialized;
use rome_js_analyze::metadata;
use rome_json_formatter::context::JsonFormatOptions;
Expand Down Expand Up @@ -208,7 +208,7 @@ pub fn load_config(
);
}

let deserialized = deserialize_from_json::<Configuration>(&buffer)
let deserialized = deserialize_from_json_str::<Configuration>(&buffer)
.with_file_path(&configuration_path.display().to_string());
Ok(Some(deserialized))
}
Expand Down
21 changes: 18 additions & 3 deletions crates/rome_service/src/file_handlers/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ use crate::settings::{
FormatSettings, Language, LanguageSettings, LanguagesSettings, SettingsHandle,
};
use crate::workspace::{GetSyntaxTreeResult, PullActionsResult};
use crate::{Rules, WorkspaceError};
use crate::{Configuration, Rules, WorkspaceError};
use rome_deserialize::json::deserialize_from_json_ast;
use rome_diagnostics::{Diagnostic, Severity};
use rome_formatter::{FormatError, Printed};
use rome_fs::RomePath;
use rome_fs::{RomePath, CONFIG_NAME};
use rome_json_formatter::context::JsonFormatOptions;
use rome_json_formatter::format_node;
use rome_json_syntax::{JsonLanguage, JsonRoot, JsonSyntaxNode};
Expand Down Expand Up @@ -176,7 +177,21 @@ fn format_on_type(
}

fn lint(params: LintParams) -> LintResults {
let diagnostics = params.parse.into_diagnostics();
let root: JsonRoot = params.parse.tree();
let mut diagnostics = params.parse.into_diagnostics();

// if we're parsing the `rome.json` file, we deserialize it, so we can emit diagnostics for
// malformed configuration
if params.path.ends_with(CONFIG_NAME) {
let deserialized = deserialize_from_json_ast::<Configuration>(root);
diagnostics.extend(
deserialized
.into_diagnostics()
.into_iter()
.map(rome_diagnostics::serde::Diagnostic::new)
.collect::<Vec<_>>(),
);
}

let diagnostic_count = diagnostics.len() as u64;
let errors = diagnostics
Expand Down
1 change: 1 addition & 0 deletions crates/rome_service/src/file_handlers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ pub(crate) struct LintParams<'a> {
pub(crate) rules: Option<&'a Rules>,
pub(crate) settings: SettingsHandle<'a>,
pub(crate) max_diagnostics: u64,
pub(crate) path: &'a RomePath,
}

pub(crate) struct LintResults {
Expand Down
1 change: 1 addition & 0 deletions crates/rome_service/src/workspace/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ impl Workspace for WorkspaceServer {
rules,
settings: self.settings(),
max_diagnostics: params.max_diagnostics,
path: &params.path,
});

(
Expand Down
4 changes: 2 additions & 2 deletions crates/rome_service/tests/spec_tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rome_deserialize::json::deserialize_from_json;
use rome_deserialize::json::deserialize_from_json_str;
use rome_diagnostics::{print_diagnostic_to_string, DiagnosticExt};
use rome_service::Configuration;
use std::ffi::OsStr;
Expand All @@ -14,7 +14,7 @@ fn run_invalid_configurations(input: &'static str, _: &str, _: &str, _: &str) {
.unwrap_or_else(|err| panic!("failed to read {:?}: {:?}", input_file, err));

let result = match extension {
"json" => deserialize_from_json::<Configuration>(input_code.as_str()),
"json" => deserialize_from_json_str::<Configuration>(input_code.as_str()),
_ => {
panic!("Extension not supported");
}
Expand Down

0 comments on commit af991f4

Please sign in to comment.