Skip to content

Commit

Permalink
feat(lint): use default globs, upgrade to v0.1.9 (denoland#6222)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bartlomieju committed Jun 10, 2020
1 parent e53a1b1 commit e4e332a
Show file tree
Hide file tree
Showing 12 changed files with 165 additions and 101 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
45 changes: 38 additions & 7 deletions cli/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
}

Expand Down Expand Up @@ -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
Expand All @@ -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),
)
}

Expand Down Expand Up @@ -1640,6 +1640,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"]);
Expand Down
37 changes: 21 additions & 16 deletions cli/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
79 changes: 79 additions & 0 deletions cli/lint.rs
Original file line number Diff line number Diff line change
@@ -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,
)
}
54 changes: 3 additions & 51 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ mod import_map;
mod inspector;
pub mod installer;
mod js;
mod lint;
mod lockfile;
mod metrics;
mod module_graph;
Expand Down Expand Up @@ -321,64 +322,15 @@ 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,
true,
)?;

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> {
Expand Down
6 changes: 6 additions & 0 deletions cli/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
26 changes: 8 additions & 18 deletions cli/tests/lint/expected.out
Original file line number Diff line number Diff line change
@@ -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) {}
Expand All @@ -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
2 changes: 2 additions & 0 deletions cli/tests/lint/expected_glob.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[WILDCARD]
Found 5 problems
5 changes: 2 additions & 3 deletions cli/tests/lint/file1.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
var a = 1,
b = 2,
c = 3;
// deno-lint-ignore
while (false) {}
3 changes: 0 additions & 3 deletions cli/tests/lint/file2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,3 @@ try {

// deno-lint-ignore no-explicit-any require-await
function foo(): any {}

// deno-lint-ignore
while (false) {}
3 changes: 3 additions & 0 deletions cli/tests/std_lint.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[WILDCARD]

Found [WILDCARD] problems

0 comments on commit e4e332a

Please sign in to comment.