diff options
author | Bartek IwaĆczuk <biwanczuk@gmail.com> | 2020-06-10 23:29:48 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-06-10 23:29:48 +0200 |
commit | e4e332abbbe0dbdb44305a261f9965ba89e7767b (patch) | |
tree | 6bdbeb2c6d1039adf34f7366e467e105e91ab63d | |
parent | e53a1b14968099df06624a4dc4d4ab810d1ce443 (diff) |
feat(lint): use default globs, upgrade to v0.1.9 (#6222)
This commit:
* added default file globs so "deno lint" can be run
without arguments (just like "deno fmt")
* added test for globs in "deno lint"
* upgrade "deno_lint" crate to v0.1.9
-rw-r--r-- | Cargo.lock | 4 | ||||
-rw-r--r-- | cli/Cargo.toml | 2 | ||||
-rw-r--r-- | cli/flags.rs | 45 | ||||
-rw-r--r-- | cli/fmt.rs | 37 | ||||
-rw-r--r-- | cli/lint.rs | 79 | ||||
-rw-r--r-- | cli/main.rs | 54 | ||||
-rw-r--r-- | cli/tests/integration_tests.rs | 6 | ||||
-rw-r--r-- | cli/tests/lint/expected.out | 26 | ||||
-rw-r--r-- | cli/tests/lint/expected_glob.out | 2 | ||||
-rw-r--r-- | cli/tests/lint/file1.js | 5 | ||||
-rw-r--r-- | cli/tests/lint/file2.ts | 3 | ||||
-rw-r--r-- | cli/tests/std_lint.out | 3 |
12 files changed, 165 insertions, 101 deletions
diff --git a/Cargo.lock b/Cargo.lock index eb3349c1f..cdfd43d5c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -506,9 +506,9 @@ dependencies = [ [[package]] name = "deno_lint" -version = "0.1.8" +version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ca00aa150fff66457af99578360267a988b44b3dd0d7d503571b2e98b15094d" +checksum = "886dbbba1d26b726e4e6c4c03001b7f21b31384acb2a038e5bce430c42de4190" dependencies = [ "lazy_static", "regex", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 667a6e2c3..f8ad6abb9 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -20,7 +20,7 @@ deno_typescript = { path = "../deno_typescript", version = "0.47.1" } [dependencies] deno_core = { path = "../core", version = "0.47.1" } -deno_lint = { version = "0.1.8" } +deno_lint = { version = "0.1.9" } deno_typescript = { path = "../deno_typescript", version = "0.47.1" } atty = "0.2.14" diff --git a/cli/flags.rs b/cli/flags.rs index f630f4bbf..acfe2f3e9 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -588,11 +588,10 @@ fn doc_parse(flags: &mut Flags, matches: &clap::ArgMatches) { fn lint_parse(flags: &mut Flags, matches: &clap::ArgMatches) { unstable_arg_parse(flags, matches); - let files = matches - .values_of("files") - .unwrap() - .map(String::from) - .collect(); + let files = match matches.values_of("files") { + Some(f) => f.map(String::from).collect(), + None => vec![], + }; flags.subcommand = DenoSubcommand::Lint { files }; } @@ -911,6 +910,7 @@ fn lint_subcommand<'a, 'b>() -> App<'a, 'b> { .about("Lint source files") .long_about( "Lint JavaScript/TypeScript source code. + deno lint --unstable deno lint --unstable myfile1.ts myfile2.js Ignore diagnostics on the next line by preceding it with an ignore comment and @@ -927,8 +927,8 @@ Ignore linting a file by adding an ignore comment at the top of the file: .arg( Arg::with_name("files") .takes_value(true) - .required(true) - .min_values(1), + .multiple(true) + .required(false), ) } @@ -1641,6 +1641,37 @@ mod tests { } #[test] + fn lint() { + let r = flags_from_vec_safe(svec![ + "deno", + "lint", + "--unstable", + "script_1.ts", + "script_2.ts" + ]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Lint { + files: vec!["script_1.ts".to_string(), "script_2.ts".to_string()] + }, + unstable: true, + ..Flags::default() + } + ); + + let r = flags_from_vec_safe(svec!["deno", "lint", "--unstable"]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Lint { files: vec![] }, + unstable: true, + ..Flags::default() + } + ); + } + + #[test] fn types() { let r = flags_from_vec_safe(svec!["deno", "types"]); assert_eq!( diff --git a/cli/fmt.rs b/cli/fmt.rs index 0d7a909bc..229039606 100644 --- a/cli/fmt.rs +++ b/cli/fmt.rs @@ -34,23 +34,8 @@ pub async fn format(args: Vec<String>, check: bool) -> Result<(), ErrBox> { return format_stdin(check); } - let mut target_files: Vec<PathBuf> = vec![]; + let target_files = collect_files(args)?; - if args.is_empty() { - target_files.extend(files_in_subtree( - std::env::current_dir().unwrap(), - is_supported, - )); - } else { - for arg in args { - let p = PathBuf::from(arg); - if p.is_dir() { - target_files.extend(files_in_subtree(p, is_supported)); - } else { - target_files.push(p); - }; - } - } let config = get_config(); if check { check_source_files(config, target_files).await @@ -222,6 +207,26 @@ fn is_supported(path: &Path) -> bool { } } +pub fn collect_files(files: Vec<String>) -> Result<Vec<PathBuf>, ErrBox> { + let mut target_files: Vec<PathBuf> = vec![]; + + if files.is_empty() { + target_files + .extend(files_in_subtree(std::env::current_dir()?, is_supported)); + } else { + for arg in files { + let p = PathBuf::from(arg); + if p.is_dir() { + target_files.extend(files_in_subtree(p, is_supported)); + } else { + target_files.push(p); + }; + } + } + + Ok(target_files) +} + fn get_config() -> dprint::configuration::Configuration { use dprint::configuration::*; ConfigurationBuilder::new().deno().build() diff --git a/cli/lint.rs b/cli/lint.rs new file mode 100644 index 000000000..5dfbcaf40 --- /dev/null +++ b/cli/lint.rs @@ -0,0 +1,79 @@ +// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. + +//! This module provides file formating utilities using +//! [`deno_lint`](https://github.com/denoland/deno_lint). +//! +//! At the moment it is only consumed using CLI but in +//! the future it can be easily extended to provide +//! the same functions as ops available in JS runtime. + +use crate::colors; +use crate::file_fetcher::map_file_extension; +use crate::fmt::collect_files; +use crate::fmt_errors; +use crate::swc_util; +use deno_core::ErrBox; +use deno_lint::diagnostic::LintDiagnostic; +use deno_lint::linter::Linter; +use deno_lint::rules; +use std::fs; +use std::path::PathBuf; + +pub fn lint_files(args: Vec<String>) -> Result<(), ErrBox> { + let target_files = collect_files(args)?; + + let mut error_counts = 0; + + for file_path in target_files { + let file_diagnostics = lint_file(file_path)?; + error_counts += file_diagnostics.len(); + for d in file_diagnostics.iter() { + let fmt_diagnostic = format_diagnostic(d); + eprintln!("{}\n", fmt_diagnostic); + } + } + + if error_counts > 0 { + eprintln!("Found {} problems", error_counts); + std::process::exit(1); + } + + Ok(()) +} + +fn lint_file(file_path: PathBuf) -> Result<Vec<LintDiagnostic>, ErrBox> { + let file_name = file_path.to_string_lossy().to_string(); + let source_code = fs::read_to_string(&file_path)?; + let media_type = map_file_extension(&file_path); + let syntax = swc_util::get_syntax_for_media_type(media_type); + + let mut linter = Linter::default(); + let lint_rules = rules::get_recommended_rules(); + + let file_diagnostics = + linter.lint(file_name, source_code, syntax, lint_rules)?; + + Ok(file_diagnostics) +} + +fn format_diagnostic(d: &LintDiagnostic) -> String { + let pretty_message = format!( + "({}) {}", + colors::gray(d.code.to_string()), + d.message.clone() + ); + + fmt_errors::format_stack( + true, + pretty_message, + Some(d.line_src.clone()), + Some(d.location.col as i64), + Some((d.location.col + d.snippet_length) as i64), + &[fmt_errors::format_location( + d.location.filename.clone(), + d.location.line as i64, + d.location.col as i64, + )], + 0, + ) +} diff --git a/cli/main.rs b/cli/main.rs index d3be3bdbe..73349d599 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -42,6 +42,7 @@ mod import_map; mod inspector; pub mod installer; mod js; +mod lint; mod lockfile; mod metrics; mod module_graph; @@ -321,7 +322,7 @@ async fn lint_command(flags: Flags, files: Vec<String>) -> Result<(), ErrBox> { // state just to perform unstable check... use crate::state::State; let state = State::new( - global_state.clone(), + global_state, None, ModuleSpecifier::resolve_url("file:///dummy.ts").unwrap(), None, @@ -329,56 +330,7 @@ async fn lint_command(flags: Flags, files: Vec<String>) -> Result<(), ErrBox> { )?; state.check_unstable("lint"); - - let mut error_counts = 0; - - for file in files { - let specifier = ModuleSpecifier::resolve_url_or_path(&file)?; - let source_file = global_state - .file_fetcher - .fetch_source_file(&specifier, None, Permissions::allow_all()) - .await?; - let source_code = String::from_utf8(source_file.source_code)?; - let syntax = swc_util::get_syntax_for_media_type(source_file.media_type); - - let mut linter = deno_lint::linter::Linter::default(); - let lint_rules = deno_lint::rules::get_all_rules(); - - let file_diagnostics = - linter.lint(file, source_code, syntax, lint_rules)?; - - error_counts += file_diagnostics.len(); - for d in file_diagnostics.iter() { - let pretty_message = format!( - "({}) {}", - colors::gray(d.code.to_string()), - d.message.clone() - ); - eprintln!( - "{}\n", - fmt_errors::format_stack( - true, - pretty_message, - Some(d.line_src.clone()), - Some(d.location.col as i64), - Some((d.location.col + d.snippet_length) as i64), - &[fmt_errors::format_location( - d.location.filename.clone(), - d.location.line as i64, - d.location.col as i64, - )], - 0 - ) - ); - } - } - - if error_counts > 0 { - eprintln!("Found {} problems", error_counts); - std::process::exit(1); - } - - Ok(()) + lint::lint_files(files) } async fn cache_command(flags: Flags, files: Vec<String>) -> Result<(), ErrBox> { diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index fafce3eb0..e3f4bd085 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -1957,6 +1957,12 @@ itest!(deno_lint { exit_code: 1, }); +itest!(deno_lint_glob { + args: "lint --unstable lint/", + output: "lint/expected_glob.out", + exit_code: 1, +}); + #[test] fn cafile_fetch() { use url::Url; diff --git a/cli/tests/lint/expected.out b/cli/tests/lint/expected.out index 0b0654298..57af15971 100644 --- a/cli/tests/lint/expected.out +++ b/cli/tests/lint/expected.out @@ -1,12 +1,12 @@ -(no-var) `var` keyword is not allowed -var a = 1, -~~~~~~~~~~ +(ban-untagged-ignore) Ignore directive requires lint rule code +// deno-lint-ignore +~~~~~~~~~~~~~~~~~~~ at [WILDCARD]file1.js:1:0 -(single-var-declarator) Multiple variable declarators are not allowed -var a = 1, -~~~~~~~~~~ - at [WILDCARD]file1.js:1:0 +(no-empty) Empty block statement +while (false) {} + ~~ + at [WILDCARD]file1.js:2:14 (no-empty) Empty block statement } catch (e) {} @@ -23,14 +23,4 @@ function foo(): any {} ~~~~~~~~~~~~~~~~~~~~~~ at [WILDCARD]file2.ts:6:0 -(ban-untagged-ignore) Ignore directive requires lint rule code -// deno-lint-ignore -~~~~~~~~~~~~~~~~~~~ - at [WILDCARD]file2.ts:8:0 - -(no-empty) Empty block statement -while (false) {} - ~~ - at [WILDCARD]file2.ts:9:14 - -Found 7 problems +Found 5 problems diff --git a/cli/tests/lint/expected_glob.out b/cli/tests/lint/expected_glob.out new file mode 100644 index 000000000..3e3ebd649 --- /dev/null +++ b/cli/tests/lint/expected_glob.out @@ -0,0 +1,2 @@ +[WILDCARD] +Found 5 problems diff --git a/cli/tests/lint/file1.js b/cli/tests/lint/file1.js index d74b6f47a..737f26818 100644 --- a/cli/tests/lint/file1.js +++ b/cli/tests/lint/file1.js @@ -1,3 +1,2 @@ -var a = 1, - b = 2, - c = 3; +// deno-lint-ignore +while (false) {} diff --git a/cli/tests/lint/file2.ts b/cli/tests/lint/file2.ts index f0f3a3ba3..10c709ba2 100644 --- a/cli/tests/lint/file2.ts +++ b/cli/tests/lint/file2.ts @@ -4,6 +4,3 @@ try { // deno-lint-ignore no-explicit-any require-await function foo(): any {} - -// deno-lint-ignore -while (false) {} diff --git a/cli/tests/std_lint.out b/cli/tests/std_lint.out new file mode 100644 index 000000000..9d62fcc67 --- /dev/null +++ b/cli/tests/std_lint.out @@ -0,0 +1,3 @@ +[WILDCARD] + +Found [WILDCARD] problems
\ No newline at end of file |