Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(grit): add auto-wrap + integrate with search command #3288

Merged
merged 7 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion crates/biome_cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ pub enum BiomeCommand {
/// The GritQL pattern to search for.
///
/// Note that the search command (currently) does not support rewrites.
#[bpaf(positional("PATH"))]
#[bpaf(positional("PATTERN"))]
pattern: String,

/// Single file, single path or list of paths.
Expand Down
5 changes: 3 additions & 2 deletions crates/biome_cli/src/execute/traverse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ pub(crate) fn traverse(
TraversalMode::Check { .. }
| TraversalMode::Lint { .. }
| TraversalMode::Format { .. }
| TraversalMode::CI { .. } => {
| TraversalMode::CI { .. }
| TraversalMode::Search { .. } => {
// If `--staged` or `--changed` is specified, it's acceptable for them to be empty, so ignore it.
if !execution.is_vcs_targeted() {
match current_dir() {
Expand Down Expand Up @@ -583,7 +584,7 @@ impl<'ctx, 'app> TraversalContext for TraversalOptions<'ctx, 'app> {
TraversalMode::Lint { .. } => file_features.supports_lint(),
// Imagine if Biome can't handle its own configuration file...
TraversalMode::Migrate { .. } => true,
TraversalMode::Search { .. } => false,
TraversalMode::Search { .. } => file_features.supports_search(),
}
}

Expand Down
13 changes: 5 additions & 8 deletions crates/biome_css_formatter/tests/language.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,11 @@ impl TestFormatLanguage for CssTestFormatLanguage {
type FormatLanguage = CssFormatLanguage;

fn parse(&self, text: &str) -> AnyParse {
let parse = parse_css(
text,
CssParserOptions::default()
.allow_wrong_line_comments()
.allow_css_modules(),
);

AnyParse::new(parse.syntax().as_send().unwrap(), parse.into_diagnostics())
let options = CssParserOptions::default()
.allow_wrong_line_comments()
.allow_css_modules();

parse_css(text, options).into()
}

fn to_language_settings<'a>(
Expand Down
14 changes: 13 additions & 1 deletion crates/biome_css_parser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::syntax::parse_root;
use biome_css_factory::CssSyntaxFactory;
use biome_css_syntax::{CssLanguage, CssRoot, CssSyntaxNode};
pub use biome_parser::prelude::*;
use biome_parser::tree_sink::LosslessTreeSink;
use biome_parser::{tree_sink::LosslessTreeSink, AnyParse};
use biome_rowan::{AstNode, NodeCache};
pub use parser::CssParserOptions;

Expand Down Expand Up @@ -107,6 +107,18 @@ impl CssParse {
}
}

impl From<CssParse> for AnyParse {
fn from(parse: CssParse) -> Self {
let root = parse.syntax();
let diagnostics = parse.into_diagnostics();
Self::new(
// SAFETY: the parser should always return a root node
root.as_send().unwrap(),
diagnostics,
)
}
}

#[cfg(test)]
mod tests {
use crate::{parse_css, CssParserOptions};
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_diagnostics/src/location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::ops::Range;
use std::{borrow::Borrow, ops::Deref};

/// Represents the location of a diagnostic in a resource.
#[derive(Debug, Clone, Copy)]
#[derive(Debug, Default, Clone, Copy)]
pub struct Location<'a> {
/// The resource this diagnostic is associated with.
pub resource: Option<Resource<&'a str>>,
Expand Down
14 changes: 13 additions & 1 deletion crates/biome_graphql_parser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use biome_graphql_factory::GraphqlSyntaxFactory;
use biome_graphql_syntax::{GraphqlLanguage, GraphqlRoot, GraphqlSyntaxNode};
pub use biome_parser::prelude::*;
use biome_parser::tree_sink::LosslessTreeSink;
use biome_parser::{tree_sink::LosslessTreeSink, AnyParse};
use biome_rowan::{AstNode, NodeCache};
use parser::{parse_root, GraphqlParser};

Expand Down Expand Up @@ -96,6 +96,18 @@ impl GraphqlParse {
}
}

impl From<GraphqlParse> for AnyParse {
fn from(parse: GraphqlParse) -> Self {
let root = parse.syntax();
let diagnostics = parse.into_diagnostics();
Self::new(
// SAFETY: the parser should always return a root node
root.as_send().unwrap(),
diagnostics,
)
}
}

#[cfg(test)]
mod tests {
use crate::parse_graphql;
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_grit_patterns/src/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use biome_diagnostics::Diagnostic;
use biome_rowan::TextRange;

#[derive(Debug, Diagnostic)]
#[derive(Clone, Debug, Diagnostic)]
#[diagnostic(severity = Warning)]
pub struct CompilerDiagnostic {
#[message]
Expand Down
132 changes: 122 additions & 10 deletions crates/biome_grit_patterns/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,31 +1,34 @@
use biome_diagnostics::serde::Diagnostic as SerializableDiagnostic;
use biome_diagnostics::Diagnostic;
use std::fmt::Debug;

use biome_console::{fmt::Formatter, markup};
use biome_diagnostics::Location;
use biome_diagnostics::{category, Category, Diagnostic, LogCategory, Severity};
use biome_parser::diagnostic::ParseDiagnostic;
use biome_rowan::SyntaxError;
use grit_util::ByteRange;
use serde::{Deserialize, Serialize};

#[derive(Debug, Deserialize, Diagnostic, Serialize)]
#[derive(Debug, Diagnostic)]
#[diagnostic(
category = "parse",
severity = Error,
message = "Error(s) parsing pattern",
)]
pub struct ParsePatternError {
diagnostics: Vec<SerializableDiagnostic>,
pub diagnostics: Vec<ParseDiagnostic>,
}

#[derive(Debug, Deserialize, Diagnostic, Serialize)]
#[derive(Debug, Diagnostic)]
#[diagnostic(
category = "parse",
severity = Error,
message = "Error(s) parsing pattern snippet",
)]
pub struct ParseSnippetError {
diagnostics: Vec<SerializableDiagnostic>,
diagnostics: Vec<ParseDiagnostic>,
}

// TODO: We definitely need to improve diagnostics.
#[derive(Debug, Deserialize, Serialize)]
#[derive(Debug)]
pub enum CompileError {
/// Indicates the (top-level) pattern could not be parsed.
ParsePatternError(ParsePatternError),
Expand Down Expand Up @@ -77,7 +80,116 @@ pub enum CompileError {
UnknownVariable(String),
}

impl Diagnostic for CompileError {}
impl Diagnostic for CompileError {
fn category(&self) -> Option<&'static Category> {
Some(category!("parse"))
}

fn message(&self, fmt: &mut Formatter<'_>) -> std::io::Result<()> {
match self {
CompileError::ParsePatternError(error) => {
fmt.write_markup(markup! { "Error parsing pattern" })?;
match error.diagnostics.first() {
Some(diag) => {
fmt.write_str(": ")?;
diag.message(fmt)
}
None => Ok(()),
Comment on lines +92 to +96
Copy link
Member

Choose a reason for hiding this comment

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

This approach may not be correct. We are basically truncating diagnostics. I think we should revisit this part.

Also, diagnostics are marked as vector of SerializableDiagnostic, but they should not, theoretically. We want to use serialisable diagnostics only in particular cases; for example, we want to cross thread boundaries inside the workspace.

This means, the transformation must happen at the workspace level, like in this case:

.map(|diag| {
let diag = diag.with_file_path(params.path.as_path().display().to_string());
SerdeDiagnostic::new(diag)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed the part about SerializableDiagnostic in dcfb43d. I'll try to revisit the diagnostics further indeed, though I hope to push that to a follow-up PR. There's some even worse feedback issues still too 😅

}
}
CompileError::ParseSnippetError(error) => {
fmt.write_markup(markup! { "Error parsing snippet" })?;
match error.diagnostics.first() {
Some(diag) => {
fmt.write_str(": ")?;
diag.message(fmt)
}
None => Ok(()),
}
}
CompileError::MissingSyntaxNode => {
fmt.write_markup(markup! { "A syntax node was missing" })
}
CompileError::DuplicateParameters => {
fmt.write_markup(markup! { "Duplicate parameters" })
}
CompileError::InvalidMetavariableRange(_) => {
fmt.write_markup(markup! { "Invalid range for metavariable" })
}
CompileError::MetavariableNotFound(var) => {
fmt.write_markup(markup! { "Metavariable not found: "{{var}} })
}
CompileError::ReservedMetavariable(var) => {
fmt.write_markup(markup! { "Reserved metavariable: "{{var}} })
}
CompileError::UnsupportedKind(kind) => {
fmt.write_markup(markup! { "Unsupported syntax kind ("{{kind}}")" })
}
CompileError::UnexpectedKind(kind) => {
fmt.write_markup(markup! { "Unexpected syntax kind ("{{kind}}")" })
}
CompileError::UnknownFunctionOrPattern(name) => {
fmt.write_markup(markup! { "Unknown function or pattern: "{{name}} })
}
CompileError::LiteralOutOfRange(value) => {
fmt.write_markup(markup! { "Literal value out of range: "{{value}} })
}
CompileError::MissingPattern => fmt.write_markup(markup! { "Missing pattern" }),
CompileError::InvalidBracketedMetavariable => {
fmt.write_markup(markup! { "Invalid bracketed metavariable" })
}
CompileError::FunctionArgument(_) => {
fmt.write_markup(markup! { "Invalid function argument" })
}
CompileError::UnknownFunctionOrPredicate(name) => {
fmt.write_markup(markup! { "Unknown function or predicate: "{{name}} })
}
CompileError::UnknownVariable(var) => {
fmt.write_markup(markup! { "Unknown variable: "{{var}} })
}
}
}

fn location(&self) -> Location<'_> {
match self {
CompileError::ParsePatternError(error) => error
.diagnostics
.first()
.map(Diagnostic::location)
.unwrap_or_default(),
_ => Location::default(),
}
}

ematipico marked this conversation as resolved.
Show resolved Hide resolved
fn description(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
CompileError::ParsePatternError(error) => match error.diagnostics.first() {
Some(diag) => diag.description(fmt),
None => Ok(()),
},
CompileError::ParseSnippetError(error) => match error.diagnostics.first() {
Some(diag) => diag.description(fmt),
None => Ok(()),
},
CompileError::FunctionArgument(error) => error.fmt(fmt),
_ => Ok(()),
}
}

fn advices(&self, visitor: &mut dyn biome_diagnostics::Visit) -> std::io::Result<()> {
match self {
CompileError::ReservedMetavariable(_) => visitor.record_log(
LogCategory::Info,
&markup! { "Try using a different variable name" }.to_owned(),
),
_ => Ok(()),
}
}

fn severity(&self) -> Severity {
Severity::Error
}
}

impl From<SyntaxError> for CompileError {
fn from(error: SyntaxError) -> Self {
Expand All @@ -93,7 +205,7 @@ impl From<NodeLikeArgumentError> for CompileError {
}
}

#[derive(Debug, Deserialize, Serialize)]
#[derive(Debug)]
pub enum NodeLikeArgumentError {
/// Duplicate arguments in invocation.
DuplicateArguments { name: String },
Expand Down
16 changes: 14 additions & 2 deletions crates/biome_grit_patterns/src/grit_binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl<'a> Binding<'a, GritQueryContext> for GritBinding<'a> {
}

fn is_suppressed(&self, _language: &GritTargetLanguage, _current_name: Option<&str>) -> bool {
todo!()
false // TODO: Implement suppression
}

fn get_insertion_padding(
Expand Down Expand Up @@ -188,7 +188,19 @@ impl<'a> Binding<'a, GritQueryContext> for GritBinding<'a> {
}

fn is_truthy(&self) -> bool {
todo!()
match self {
Self::File(_) => true,
Self::Node(node) => {
if node.is_list() {
node.has_children()
} else {
true
}
}
Self::Range(..) => true,
Self::Empty(..) => false,
Self::Constant(c) => c.is_truthy(),
}
}

fn log_empty_field_rewrite_error(
Expand Down
Loading
Loading