Refacto on lint error display in hurlfmt.

This commit is contained in:
Jean-Christophe Amiel 2024-07-10 14:13:49 +02:00
parent 497e9cae0f
commit b58bdca4e9
No known key found for this signature in database
GPG Key ID: 07FF11CFD55356CC
10 changed files with 93 additions and 114 deletions

View File

@ -20,7 +20,7 @@ def test(hurl_file):
cmd = ["hurlfmt", "--check", hurl_file] cmd = ["hurlfmt", "--check", hurl_file]
print(" ".join(cmd)) print(" ".join(cmd))
result = subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) result = subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
if result.returncode != 1: if result.returncode != 3:
print(f"return code => expected: 1 actual {result.returncode}") print(f"return code => expected: 1 actual {result.returncode}")
sys.exit(1) sys.exit(1)

View File

@ -0,0 +1,7 @@
warning: Unnecessary space
--> tests_error_lint/sections.hurl:4:1
|
4 | [QueryStringParams]
| ^ Remove space
|

View File

@ -1,4 +1,4 @@
warning: One space warning: One space
--> tests_error_lint/spaces.hurl:1:4 --> tests_error_lint/spaces.hurl:1:4
| |
1 | GET http://localhost:8000/hello 1 | GET http://localhost:8000/hello

View File

@ -15,16 +15,12 @@
* limitations under the License. * limitations under the License.
* *
*/ */
use std::path::PathBuf;
use hurl_core::error::{DisplaySourceError, OutputFormat}; use hurl_core::error::{DisplaySourceError, OutputFormat};
use hurl_core::text::{Format, Style, StyledString}; use hurl_core::text::{Format, Style, StyledString};
use crate::linter;
/// A simple logger to log app related event (start, high levels error, etc...). /// A simple logger to log app related event (start, high levels error, etc...).
pub struct Logger { pub struct Logger {
/// Format of the messaeg in the terminal: ANSI or plain. /// Format of the message in the terminal: ANSI or plain.
format: Format, format: Format,
} }
@ -44,13 +40,8 @@ impl Logger {
eprintln!("{}", s.to_string(self.format)); eprintln!("{}", s.to_string(self.format));
} }
/// Display a Hurl parsing error. /// Displays a Hurl parsing error.
pub fn error_parsing_rich<E: DisplaySourceError>( pub fn error_parsing<E: DisplaySourceError>(&self, content: &str, filename: &str, error: &E) {
&mut self,
content: &str,
filename: &str,
error: &E,
) {
// FIXME: peut-être qu'on devrait faire rentrer le prefix `error:` qui est // FIXME: peut-être qu'on devrait faire rentrer le prefix `error:` qui est
// fournit par `self.error_rich` dans la méthode `error.to_string` // fournit par `self.error_rich` dans la méthode `error.to_string`
let message = error.to_string( let message = error.to_string(
@ -67,30 +58,21 @@ impl Logger {
s.push("\n"); s.push("\n");
eprintln!("{}", s.to_string(self.format)); eprintln!("{}", s.to_string(self.format));
} }
}
pub fn make_logger_linter_error( /// Displays a lint warning.
lines: Vec<String>, pub fn warn_lint<E: DisplaySourceError>(&self, content: &str, filename: &str, error: &E) {
color: bool, let message = error.to_string(
filename: Option<PathBuf>, filename,
) -> impl Fn(&linter::Error, bool) { content,
move |error: &linter::Error, warning: bool| { None,
let filename = match &filename { OutputFormat::Terminal(self.format == Format::Ansi),
None => "-".to_string(), );
Some(value) => value.display().to_string(),
}; let mut s = StyledString::new();
let content = lines.join("\n"); s.push_with("warning", Style::new().yellow().bold());
let message = error.to_string(&filename, &content, None, OutputFormat::Terminal(color)); s.push(": ");
eprintln!("{}: {}\n", get_prefix(warning, color), message); s.push(&message);
s.push("\n");
eprintln!("{}", s.to_string(self.format));
} }
} }
fn get_prefix(warning: bool, color: bool) -> String {
let mut message = StyledString::new();
if warning {
message.push_with("warning", Style::new().yellow().bold());
} else {
message.push_with("error", Style::new().red());
};
let fmt = if color { Format::Ansi } else { Format::Plain };
message.to_string(fmt)
}

View File

@ -15,9 +15,8 @@
* limitations under the License. * limitations under the License.
* *
*/ */
pub use self::fs::read_to_string; pub use self::fs::read_to_string;
pub use self::logger::{make_logger_linter_error, Logger}; pub use self::logger::Logger;
mod fs; mod fs;
mod logger; mod logger;

View File

@ -1,31 +0,0 @@
/*
* Hurl (https://hurl.dev)
* Copyright (C) 2024 Orange
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
use hurl_core::ast::SourceInfo;
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Error {
pub source_info: SourceInfo,
pub inner: LinterError,
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum LinterError {
UnnecessarySpace,
UnnecessaryJsonEncoding,
OneSpace,
}

View File

@ -18,36 +18,46 @@
use hurl_core::ast::SourceInfo; use hurl_core::ast::SourceInfo;
use hurl_core::error; use hurl_core::error;
use hurl_core::error::DisplaySourceError; use hurl_core::error::DisplaySourceError;
use hurl_core::text::StyledString; use hurl_core::text::{Style, StyledString};
use crate::linter; #[derive(Clone, Debug, PartialEq, Eq)]
use crate::linter::LinterError; pub struct LinterError {
pub source_info: SourceInfo,
pub kind: LinterErrorKind,
}
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum LinterErrorKind {
UnnecessarySpace,
UnnecessaryJsonEncoding,
OneSpace,
}
/// ///
/// Textual Output for linter errors /// Textual Output for linter errors
/// ///
impl DisplaySourceError for linter::Error { impl DisplaySourceError for LinterError {
fn source_info(&self) -> SourceInfo { fn source_info(&self) -> SourceInfo {
self.source_info self.source_info
} }
fn description(&self) -> String { fn description(&self) -> String {
match self.inner { match self.kind {
LinterError::UnnecessarySpace => "Unnecessary space".to_string(), LinterErrorKind::UnnecessarySpace => "Unnecessary space".to_string(),
LinterError::UnnecessaryJsonEncoding => "Unnecessary json encoding".to_string(), LinterErrorKind::UnnecessaryJsonEncoding => "Unnecessary json encoding".to_string(),
LinterError::OneSpace => "One space ".to_string(), LinterErrorKind::OneSpace => "One space".to_string(),
} }
} }
fn fixme(&self, content: &[&str]) -> StyledString { fn fixme(&self, content: &[&str]) -> StyledString {
let message = match self.inner { let message = match self.kind {
LinterError::UnnecessarySpace => "Remove space".to_string(), LinterErrorKind::UnnecessarySpace => "Remove space".to_string(),
LinterError::UnnecessaryJsonEncoding => "Use Simple String".to_string(), LinterErrorKind::UnnecessaryJsonEncoding => "Use Simple String".to_string(),
LinterError::OneSpace => "Use only one space".to_string(), LinterErrorKind::OneSpace => "Use only one space".to_string(),
}; };
let mut s = StyledString::new(); let mut s = StyledString::new();
let message = error::add_carets(&message, self.source_info(), content); let message = error::add_carets(&message, self.source_info(), content);
s.push(&message); s.push_with(&message, Style::new().cyan());
s s
} }
} }

View File

@ -17,7 +17,6 @@
*/ */
pub use rules::{check_hurl_file, lint_hurl_file}; pub use rules::{check_hurl_file, lint_hurl_file};
pub use self::core::{Error, LinterError}; pub use self::error::{LinterError, LinterErrorKind};
mod core;
mod error; mod error;
mod rules; mod rules;

View File

@ -15,13 +15,12 @@
* limitations under the License. * limitations under the License.
* *
*/ */
use crate::linter::{LinterError, LinterErrorKind};
use hurl_core::ast::*; use hurl_core::ast::*;
use hurl_core::reader::Pos; use hurl_core::reader::Pos;
use crate::linter::core::{Error, LinterError};
/// Returns lint errors for the `hurl_file`. /// Returns lint errors for the `hurl_file`.
pub fn check_hurl_file(hurl_file: &HurlFile) -> Vec<Error> { pub fn check_hurl_file(hurl_file: &HurlFile) -> Vec<LinterError> {
hurl_file.entries.iter().flat_map(check_entry).collect() hurl_file.entries.iter().flat_map(check_entry).collect()
} }
@ -33,7 +32,7 @@ pub fn lint_hurl_file(hurl_file: &HurlFile) -> HurlFile {
} }
} }
fn check_entry(entry: &Entry) -> Vec<Error> { fn check_entry(entry: &Entry) -> Vec<LinterError> {
let mut errors = vec![]; let mut errors = vec![];
errors.append(&mut check_request(&entry.request)); errors.append(&mut check_request(&entry.request));
match &entry.response { match &entry.response {
@ -49,23 +48,24 @@ fn lint_entry(entry: &Entry) -> Entry {
Entry { request, response } Entry { request, response }
} }
fn check_request(request: &Request) -> Vec<Error> { fn check_request(request: &Request) -> Vec<LinterError> {
let mut errors = vec![]; let mut errors = vec![];
if !request.space0.value.is_empty() { if !request.space0.value.is_empty() {
errors.push(Error { errors.push(LinterError {
source_info: request.space0.source_info, source_info: request.space0.source_info,
inner: LinterError::UnnecessarySpace, kind: LinterErrorKind::UnnecessarySpace,
}); });
} }
if request.space1.value != " " { if request.space1.value != " " {
errors.push(Error { errors.push(LinterError {
source_info: request.space1.source_info, source_info: request.space1.source_info,
inner: LinterError::OneSpace, kind: LinterErrorKind::OneSpace,
}); });
} }
for error in check_line_terminator(&request.line_terminator0) { for error in check_line_terminator(&request.line_terminator0) {
errors.push(error); errors.push(error);
} }
errors.extend(request.sections.iter().flat_map(check_section));
errors errors
} }
@ -97,14 +97,15 @@ fn lint_request(request: &Request) -> Request {
} }
} }
fn check_response(response: &Response) -> Vec<Error> { fn check_response(response: &Response) -> Vec<LinterError> {
let mut errors = vec![]; let mut errors = vec![];
if !response.space0.value.is_empty() { if !response.space0.value.is_empty() {
errors.push(Error { errors.push(LinterError {
source_info: response.space0.source_info, source_info: response.space0.source_info,
inner: LinterError::UnnecessarySpace, kind: LinterErrorKind::UnnecessarySpace,
}); });
} }
errors.extend(response.sections.iter().flat_map(check_section));
errors errors
} }
@ -134,6 +135,20 @@ fn lint_response(response: &Response) -> Response {
} }
} }
fn check_section(section: &Section) -> Vec<LinterError> {
let mut errors = vec![];
if !section.space0.value.is_empty() {
errors.push(LinterError {
source_info: section.space0.source_info,
kind: LinterErrorKind::UnnecessarySpace,
});
}
for error in check_line_terminator(&section.line_terminator0) {
errors.push(error);
}
errors
}
fn lint_section(section: &Section) -> Section { fn lint_section(section: &Section) -> Section {
let line_terminators = section.line_terminators.clone(); let line_terminators = section.line_terminators.clone();
let line_terminator0 = section.line_terminator0.clone(); let line_terminator0 = section.line_terminator0.clone();
@ -568,15 +583,15 @@ fn one_whitespace() -> Whitespace {
} }
} }
fn check_line_terminator(line_terminator: &LineTerminator) -> Vec<Error> { fn check_line_terminator(line_terminator: &LineTerminator) -> Vec<LinterError> {
let mut errors = vec![]; let mut errors = vec![];
match &line_terminator.comment { match &line_terminator.comment {
Some(_) => {} Some(_) => {}
None => { None => {
if !line_terminator.space0.value.is_empty() { if !line_terminator.space0.value.is_empty() {
errors.push(Error { errors.push(LinterError {
source_info: line_terminator.space0.source_info, source_info: line_terminator.space0.source_info,
inner: LinterError::UnnecessarySpace, kind: LinterErrorKind::UnnecessarySpace,
}); });
} }
} }

View File

@ -27,6 +27,7 @@ use hurlfmt::{cli, curl, format, linter};
const EXIT_OK: i32 = 0; const EXIT_OK: i32 = 0;
const EXIT_ERROR: i32 = 1; const EXIT_ERROR: i32 = 1;
const EXIT_INVALID_INPUT: i32 = 2; const EXIT_INVALID_INPUT: i32 = 2;
const EXIT_LINT_ISSUE: i32 = 3;
/// Executes `hurlfmt` entry point. /// Executes `hurlfmt` entry point.
fn main() { fn main() {
@ -46,7 +47,7 @@ fn main() {
}, },
}; };
let mut logger = Logger::new(opts.color); let logger = Logger::new(opts.color);
let mut output_all = String::new(); let mut output_all = String::new();
for input_file in &opts.input_files { for input_file in &opts.input_files {
@ -63,26 +64,23 @@ fn main() {
} }
}, },
}; };
let input_path = Path::new(input_file).to_path_buf();
let lines: Vec<&str> = regex::Regex::new(r"\n|\r\n")
.unwrap()
.split(&input)
.collect();
let lines: Vec<String> = lines.iter().map(|s| (*s).to_string()).collect();
let log_linter_error =
cli::make_logger_linter_error(lines, opts.color, Some(input_path));
match parser::parse_hurl_file(&input) { match parser::parse_hurl_file(&input) {
Err(e) => { Err(e) => {
logger.error_parsing_rich(&content, input_file, &e); logger.error_parsing(&content, input_file, &e);
process::exit(EXIT_INVALID_INPUT); process::exit(EXIT_INVALID_INPUT);
} }
Ok(hurl_file) => { Ok(hurl_file) => {
if opts.check { if opts.check {
for e in linter::check_hurl_file(&hurl_file).iter() { let lints = linter::check_hurl_file(&hurl_file);
log_linter_error(e, true); for e in lints.iter() {
logger.warn_lint(&content, input_file, e);
}
if lints.is_empty() {
process::exit(EXIT_OK);
} else {
process::exit(EXIT_LINT_ISSUE);
} }
process::exit(1);
} else { } else {
let output = match opts.output_format { let output = match opts.output_format {
OutputFormat::Hurl => { OutputFormat::Hurl => {