Skip to content

Commit

Permalink
Add linting rule that is violated to linting response (#1907)
Browse files Browse the repository at this point in the history
In the past when a JSON formatted response was requested from a `lint`
command it did not include the name of the rule that was being violated,
so sometimes it was a bit challenging to see exactly what was wrong.

This PR fixes that and adds the name of the rule to the JSON output in
all the cases where linting is invoked.
  • Loading branch information
jonathanrainer authored May 30, 2024
1 parent eb16860 commit d18a549
Show file tree
Hide file tree
Showing 14 changed files with 82 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ query GraphCheckWorkflowQuery($graph_id: ID!, $workflow_id: ID!) {
level
message
coordinate
rule
sourceLocations {
start {
byteOffset
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::time::{Duration, Instant};

use graphql_client::*;

use crate::blocking::StudioClient;
use crate::operations::graph::check_workflow::types::{CheckWorkflowInput, QueryResponseData};
use crate::shared::{
Expand All @@ -8,12 +10,9 @@ use crate::shared::{
};
use crate::RoverClientError;

use graphql_client::*;

use self::graph_check_workflow_query::GraphCheckWorkflowQueryGraphCheckWorkflowTasksOn::{
LintCheckTask, OperationsCheckTask,
};

use self::graph_check_workflow_query::{
CheckWorkflowStatus, CheckWorkflowTaskStatus,
GraphCheckWorkflowQueryGraphCheckWorkflowTasksOnLintCheckTaskResult,
Expand Down Expand Up @@ -218,6 +217,7 @@ fn get_lint_response_from_result(
level: diagnostic.level.to_string(),
message: diagnostic.message,
coordinate: diagnostic.coordinate,
rule: diagnostic.rule.to_string(),
start_line,
start_byte_offset: start_byte_offset.unsigned_abs() as usize,
end_byte_offset: end_byte_offset.unsigned_abs() as usize,
Expand Down
14 changes: 11 additions & 3 deletions crates/rover-client/src/operations/graph/check_workflow/types.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use std::fmt;
use std::fmt::{Debug, Display, Formatter, Result};

use crate::operations::graph::check_workflow::runner::graph_check_workflow_query;
use crate::shared::{ChangeSeverity, CheckTaskStatus, GraphRef};
use std::fmt;

use self::graph_check_workflow_query::CheckWorkflowTaskStatus;

type QueryVariables = graph_check_workflow_query::Variables;
pub(crate) type QueryResponseData = graph_check_workflow_query::ResponseData;

use self::graph_check_workflow_query::CheckWorkflowTaskStatus;

#[derive(Debug, Clone, Eq, PartialEq)]
pub struct CheckWorkflowInput {
pub graph_ref: GraphRef,
Expand Down Expand Up @@ -71,3 +73,9 @@ impl fmt::Display for graph_check_workflow_query::LintDiagnosticLevel {
write!(f, "{}", printable)
}
}

impl Display for graph_check_workflow_query::LintRule {
fn fmt(&self, f: &mut Formatter<'_>) -> Result {
Debug::fmt(self, f)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ mutation LintGraphMutation($sdl: String!, $graphId: ID!, $baseSdl: String) {
coordinate
level
message
rule
sourceLocations {
start {
byteOffset
Expand Down
8 changes: 5 additions & 3 deletions crates/rover-client/src/operations/graph/lint/runner.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
use std::fmt;

use graphql_client::*;

use crate::blocking::StudioClient;
use crate::operations::graph::fetch;
use crate::operations::graph::fetch::GraphFetchInput;
Expand All @@ -6,9 +10,6 @@ use crate::operations::graph::lint::types::{
};
use crate::shared::{Diagnostic, GraphRef, LintResponse};
use crate::RoverClientError;
use std::fmt;

use graphql_client::*;

#[derive(GraphQLQuery)]
// The paths are relative to the directory where your `Cargo.toml` is located.
Expand Down Expand Up @@ -82,6 +83,7 @@ fn get_lint_response_from_result(
level: diagnostic.level.to_string(),
message: diagnostic.message,
coordinate: diagnostic.coordinate,
rule: diagnostic.rule.to_string(),
start_line,
start_byte_offset: start_byte_offset.unsigned_abs() as usize,
end_byte_offset: end_byte_offset.unsigned_abs() as usize,
Expand Down
8 changes: 8 additions & 0 deletions crates/rover-client/src/operations/graph/lint/types.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::fmt::{Debug, Display, Formatter, Result};

use crate::shared::GraphRef;

use super::runner::lint_graph_mutation;
Expand Down Expand Up @@ -29,3 +31,9 @@ impl From<LintGraphMutationInput> for LintQueryVariables {
}
}
}

impl Display for lint_graph_mutation::LintRule {
fn fmt(&self, f: &mut Formatter<'_>) -> Result {
Debug::fmt(self, f)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ query SubgraphCheckWorkflowQuery($workflowId: ID!, $graphId: ID!) {
level
message
coordinate
rule
sourceLocations {
start {
byteOffset
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use std::time::{Duration, Instant};

use super::types::*;
use apollo_federation_types::build::BuildError;
use graphql_client::*;

use crate::blocking::StudioClient;
use crate::operations::subgraph::check_workflow::types::QueryResponseData;
use crate::shared::{
Expand All @@ -10,22 +12,19 @@ use crate::shared::{
};
use crate::RoverClientError;

use apollo_federation_types::build::BuildError;
use super::types::*;

use self::subgraph_check_workflow_query::SubgraphCheckWorkflowQueryGraphCheckWorkflowTasksOn::{
CompositionCheckTask, DownstreamCheckTask, LintCheckTask, OperationsCheckTask,
ProposalsCheckTask,
};

use self::subgraph_check_workflow_query::{
CheckWorkflowStatus, CheckWorkflowTaskStatus, ProposalStatus,
SubgraphCheckWorkflowQueryGraphCheckWorkflowTasksOnDownstreamCheckTaskResults,
SubgraphCheckWorkflowQueryGraphCheckWorkflowTasksOnLintCheckTaskResult,
SubgraphCheckWorkflowQueryGraphCheckWorkflowTasksOnOperationsCheckTaskResult,
};

use graphql_client::*;

#[derive(GraphQLQuery)]
// The paths are relative to the directory where your `Cargo.toml` is located.
// Both json and the GraphQL schema language are supported as sources for the schema
Expand Down Expand Up @@ -305,6 +304,7 @@ fn get_lint_response_from_result(
level: diagnostic.level.to_string(),
message: diagnostic.message,
coordinate: diagnostic.coordinate,
rule: diagnostic.rule.to_string(),
start_line,
start_byte_offset: start_byte_offset.unsigned_abs() as usize,
end_byte_offset: end_byte_offset.unsigned_abs() as usize,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
use core::fmt;
use std::fmt::{Debug, Display, Formatter, Result};

use crate::operations::subgraph::check_workflow::runner::subgraph_check_workflow_query;
use crate::shared::CheckTaskStatus;
use crate::shared::{ChangeSeverity, GraphRef};
use core::fmt;

use self::subgraph_check_workflow_query::CheckWorkflowTaskStatus;

type QueryVariables = subgraph_check_workflow_query::Variables;
pub(crate) type QueryResponseData = subgraph_check_workflow_query::ResponseData;

pub type ProposalsCheckTaskUnion = self::subgraph_check_workflow_query::SubgraphCheckWorkflowQueryGraphCheckWorkflowTasksOnProposalsCheckTask;

use self::subgraph_check_workflow_query::CheckWorkflowTaskStatus;
use crate::shared::CheckTaskStatus;

#[derive(Debug, Clone, Eq, PartialEq)]
pub struct CheckWorkflowInput {
pub graph_ref: GraphRef,
Expand Down Expand Up @@ -74,3 +76,9 @@ impl fmt::Display for subgraph_check_workflow_query::LintDiagnosticLevel {
write!(f, "{}", printable)
}
}

impl Display for subgraph_check_workflow_query::LintRule {
fn fmt(&self, f: &mut Formatter<'_>) -> Result {
Debug::fmt(self, f)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ mutation LintSubgraphMutation($sdl: String!, $graphId: ID!, $baseSdl: String) {
coordinate
level
message
rule
sourceLocations {
start {
byteOffset
Expand Down
5 changes: 3 additions & 2 deletions crates/rover-client/src/operations/subgraph/lint/runner.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::fmt;

use graphql_client::*;

use crate::blocking::StudioClient;
use crate::operations::config::is_federated::{self, IsFederatedInput};
use crate::operations::subgraph::fetch;
Expand All @@ -10,8 +12,6 @@ use crate::operations::subgraph::lint::types::{
use crate::shared::{Diagnostic, GraphRef, LintResponse};
use crate::RoverClientError;

use graphql_client::*;

#[derive(GraphQLQuery)]
// The paths are relative to the directory where your `Cargo.toml` is located.
// Both json and the GraphQL schema language are supported as sources for the schema
Expand Down Expand Up @@ -101,6 +101,7 @@ fn get_lint_response_from_result(
level: diagnostic.level.to_string(),
message: diagnostic.message,
coordinate: diagnostic.coordinate,
rule: diagnostic.rule.to_string(),
start_line,
start_byte_offset: start_byte_offset.unsigned_abs() as usize,
end_byte_offset: end_byte_offset.unsigned_abs() as usize,
Expand Down
8 changes: 8 additions & 0 deletions crates/rover-client/src/operations/subgraph/lint/types.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::fmt::{Debug, Display, Formatter, Result};

use crate::shared::GraphRef;

use super::runner::lint_subgraph_mutation;
Expand Down Expand Up @@ -30,3 +32,9 @@ impl From<LintSubgraphMutationInput> for LintQueryVariables {
}
}
}

impl Display for lint_subgraph_mutation::LintRule {
fn fmt(&self, f: &mut Formatter<'_>) -> Result {
Debug::fmt(self, f)
}
}
2 changes: 2 additions & 0 deletions crates/rover-client/src/shared/lint_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ pub struct Diagnostic {
pub start_line: i64,
pub start_byte_offset: usize,
pub end_byte_offset: usize,
pub rule: String,
}

#[cfg(test)]
Expand All @@ -136,6 +137,7 @@ type Query {
level: "WARNING".to_string(),
coordinate: "Query.key".to_string(),
message: "Schema element Query.key is missing a description.".to_string(),
rule: "DESCRIPTION_MISSING".to_string(),
start_line: 3,
start_byte_offset: 50,
end_byte_offset: 53,
Expand Down
34 changes: 21 additions & 13 deletions src/command/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,11 @@ use std::{
io::{self, IsTerminal},
};

use crate::command::supergraph::compose::CompositionOutput;
use crate::options::JsonVersion;
use crate::utils::table::{self, row};
use crate::RoverError;

use crate::command::template::queries::list_templates_for_language::ListTemplatesForLanguageTemplates;
use crate::options::ProjectLanguage;
use calm_io::{stderr, stderrln};
use camino::Utf8PathBuf;
use serde_json::{json, Value};
use termimad::{crossterm::style::Attribute::Underlined, MadSkin};

use rover_client::operations::contract::describe::ContractDescribeResponse;
use rover_client::operations::contract::publish::ContractPublishResponse;
use rover_client::operations::graph::publish::GraphPublishResponse;
Expand All @@ -26,8 +22,13 @@ use rover_client::shared::{
};
use rover_client::RoverClientError;
use rover_std::Style;
use serde_json::{json, Value};
use termimad::{crossterm::style::Attribute::Underlined, MadSkin};

use crate::command::supergraph::compose::CompositionOutput;
use crate::command::template::queries::list_templates_for_language::ListTemplatesForLanguageTemplates;
use crate::options::JsonVersion;
use crate::options::ProjectLanguage;
use crate::utils::table::{self, row};
use crate::RoverError;

/// RoverOutput defines all of the different types of data that are printed
/// to `stdout`. Every one of Rover's commands should return `saucer::Result<RoverOutput>`
Expand Down Expand Up @@ -650,8 +651,11 @@ impl RoverOutput {
mod tests {
use std::collections::BTreeMap;

use anyhow::anyhow;
use apollo_federation_types::build::{BuildError, BuildErrors};
use assert_json_diff::assert_json_eq;
use chrono::{DateTime, Local, Utc};

use rover_client::{
operations::{
graph::publish::{ChangeSummary, FieldChanges, TypeChanges},
Expand All @@ -668,10 +672,6 @@ mod tests {
},
};

use apollo_federation_types::build::{BuildError, BuildErrors};

use anyhow::anyhow;

use crate::options::JsonOutput;

use super::*;
Expand Down Expand Up @@ -975,6 +975,7 @@ mod tests {
target_url: Some("https://studio.apollographql.com/graph/my-graph/variant/current/lint/1".to_string()),
diagnostics: vec![
Diagnostic {
rule: "FIELD_NAMES_SHOULD_BE_CAMEL_CASE".to_string(),
level: "WARNING".to_string(),
message: "Field must be camelCase.".to_string(),
coordinate: "Query.all_users".to_string(),
Expand Down Expand Up @@ -1037,6 +1038,7 @@ mod tests {
"start_line": 1,
"start_byte_offset": 4,
"end_byte_offset": 2,
"rule": "FIELD_NAMES_SHOULD_BE_CAMEL_CASE"
}
],
"errors_count": 0,
Expand Down Expand Up @@ -1096,6 +1098,7 @@ mod tests {
),
diagnostics: vec![
Diagnostic {
rule: "FIELD_NAMES_SHOULD_BE_CAMEL_CASE".to_string(),
level: "WARNING".to_string(),
message: "Field must be camelCase.".to_string(),
coordinate: "Query.all_users".to_string(),
Expand All @@ -1104,6 +1107,7 @@ mod tests {
end_byte_offset: 0
},
Diagnostic {
rule: "TYPE_NAMES_SHOULD_BE_PASCAL_CASE".to_string(),
level: "ERROR".to_string(),
message: "Type name must be PascalCase.".to_string(),
coordinate: "userContext".to_string(),
Expand Down Expand Up @@ -1169,6 +1173,7 @@ mod tests {
"start_line": 2,
"start_byte_offset": 8,
"end_byte_offset": 0,
"rule": "FIELD_NAMES_SHOULD_BE_CAMEL_CASE"
},
{
"level": "ERROR",
Expand All @@ -1177,6 +1182,7 @@ mod tests {
"start_line": 3,
"start_byte_offset": 5,
"end_byte_offset": 0,
"rule": "TYPE_NAMES_SHOULD_BE_PASCAL_CASE"
}
],
"errors_count": 1,
Expand Down Expand Up @@ -1568,6 +1574,7 @@ mod tests {
let actual_json: JsonOutput = RoverError::new(RoverClientError::LintFailures {
lint_response: LintResponse {
diagnostics: [Diagnostic {
rule: "FIELD_NAMES_SHOULD_BE_CAMEL_CASE".to_string(),
coordinate: "Query.Hello".to_string(),
level: "ERROR".to_string(),
message: "Field names should use camelCase style.".to_string(),
Expand All @@ -1593,6 +1600,7 @@ mod tests {
"start_line": 0,
"start_byte_offset": 13,
"end_byte_offset": 18,
"rule": "FIELD_NAMES_SHOULD_BE_CAMEL_CASE"
}
],
"file_name": "/tmp/schema.graphql",
Expand Down

0 comments on commit d18a549

Please sign in to comment.