1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-21 15:04:11 -05:00

feat(lint): add support for config file and CLI flags for rules (#11776)

This commit adds support for following flags in deno lint subcommand:

--config - allows to load configuration file and parses "lint" object
--rules-tags=<tags> - allows specifying which set of tagged rules should be run
--rules-include=<rules> - allow specifying which rules should be run
--rules-exclude=<rules> - allow specifying which rules should not be run
This commit is contained in:
Bartek Iwańczuk 2021-09-03 17:01:58 +02:00 committed by GitHub
parent c3001fe280
commit d93570a619
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 484 additions and 31 deletions

View file

@ -234,7 +234,7 @@ impl TsConfig {
maybe_config_file: Option<&ConfigFile>,
) -> Result<Option<IgnoredCompilerOptions>, AnyError> {
if let Some(config_file) = maybe_config_file {
let (value, maybe_ignored_options) = config_file.as_compiler_options()?;
let (value, maybe_ignored_options) = config_file.to_compiler_options()?;
self.merge(&value);
Ok(maybe_ignored_options)
} else {
@ -266,10 +266,33 @@ impl Serialize for TsConfig {
}
}
#[derive(Clone, Debug, Default, Deserialize)]
#[serde(default, deny_unknown_fields)]
pub struct LintRulesConfig {
pub tags: Option<Vec<String>>,
pub include: Option<Vec<String>>,
pub exclude: Option<Vec<String>>,
}
#[derive(Clone, Debug, Default, Deserialize)]
#[serde(default, deny_unknown_fields)]
pub struct LintFilesConfig {
pub include: Vec<String>,
pub exclude: Vec<String>,
}
#[derive(Clone, Debug, Default, Deserialize)]
#[serde(default, deny_unknown_fields)]
pub struct LintConfig {
pub rules: LintRulesConfig,
pub files: LintFilesConfig,
}
#[derive(Clone, Debug, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct ConfigFileJson {
pub compiler_options: Option<Value>,
pub lint: Option<Value>,
}
#[derive(Clone, Debug)]
@ -328,7 +351,7 @@ impl ConfigFile {
/// Parse `compilerOptions` and return a serde `Value`.
/// The result also contains any options that were ignored.
pub fn as_compiler_options(
pub fn to_compiler_options(
&self,
) -> Result<(Value, Option<IgnoredCompilerOptions>), AnyError> {
if let Some(compiler_options) = self.json.compiler_options.clone() {
@ -340,6 +363,16 @@ impl ConfigFile {
Ok((json!({}), None))
}
}
pub fn to_lint_config(&self) -> Result<Option<LintConfig>, AnyError> {
if let Some(config) = self.json.lint.clone() {
let lint_config: LintConfig = serde_json::from_value(config)
.context("Failed to parse \"lint\" configuration")?;
Ok(Some(lint_config))
} else {
Ok(None)
}
}
}
#[cfg(test)]
@ -397,12 +430,22 @@ mod tests {
"build": true,
// comments are allowed
"strict": true
},
"lint": {
"files": {
"include": ["src/"],
"exclude": ["src/testdata/"]
},
"rules": {
"tags": ["recommended"],
"include": ["ban-untagged-todo"]
}
}
}"#;
let config_path = PathBuf::from("/deno/tsconfig.json");
let config_file = ConfigFile::new(config_text, &config_path).unwrap();
let (options_value, ignored) =
config_file.as_compiler_options().expect("error parsing");
config_file.to_compiler_options().expect("error parsing");
assert!(options_value.is_object());
let options = options_value.as_object().unwrap();
assert!(options.contains_key("strict"));
@ -414,6 +457,22 @@ mod tests {
maybe_path: Some(config_path),
}),
);
let lint_config = config_file
.to_lint_config()
.expect("error parsing lint object")
.expect("lint object should be defined");
assert_eq!(lint_config.files.include, vec!["src/"]);
assert_eq!(lint_config.files.exclude, vec!["src/testdata/"]);
assert_eq!(
lint_config.rules.include,
Some(vec!["ban-untagged-todo".to_string()])
);
assert_eq!(
lint_config.rules.tags,
Some(vec!["recommended".to_string()])
);
assert!(lint_config.rules.exclude.is_none());
}
#[test]
@ -422,7 +481,7 @@ mod tests {
let config_path = PathBuf::from("/deno/tsconfig.json");
let config_file = ConfigFile::new(config_text, &config_path).unwrap();
let (options_value, _) =
config_file.as_compiler_options().expect("error parsing");
config_file.to_compiler_options().expect("error parsing");
assert!(options_value.is_object());
}
@ -432,7 +491,7 @@ mod tests {
let config_path = PathBuf::from("/deno/tsconfig.json");
let config_file = ConfigFile::new(config_text, &config_path).unwrap();
let (options_value, _) =
config_file.as_compiler_options().expect("error parsing");
config_file.to_compiler_options().expect("error parsing");
assert!(options_value.is_object());
}

View file

@ -90,6 +90,9 @@ pub enum DenoSubcommand {
files: Vec<PathBuf>,
ignore: Vec<PathBuf>,
rules: bool,
rules_tags: Vec<String>,
rules_include: Vec<String>,
rules_exclude: Vec<String>,
json: bool,
},
Repl {
@ -952,6 +955,35 @@ Ignore linting a file by adding an ignore comment at the top of the file:
.long("rules")
.help("List available rules"),
)
.arg(
Arg::with_name("rules-tags")
.long("rules-tags")
.require_equals(true)
.takes_value(true)
.use_delimiter(true)
.empty_values(true)
.conflicts_with("rules")
.help("Use set of rules with a tag"),
)
.arg(
Arg::with_name("rules-include")
.long("rules-include")
.require_equals(true)
.takes_value(true)
.use_delimiter(true)
.conflicts_with("rules")
.help("Include lint rules"),
)
.arg(
Arg::with_name("rules-exclude")
.long("rules-exclude")
.require_equals(true)
.takes_value(true)
.use_delimiter(true)
.conflicts_with("rules")
.help("Exclude lint rules"),
)
.arg(config_arg())
.arg(
Arg::with_name("ignore")
.long("ignore")
@ -1722,6 +1754,7 @@ fn lsp_parse(flags: &mut Flags, _matches: &clap::ArgMatches) {
}
fn lint_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
config_arg_parse(flags, matches);
let files = match matches.values_of("files") {
Some(f) => f.map(PathBuf::from).collect(),
None => vec![],
@ -1731,10 +1764,25 @@ fn lint_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
None => vec![],
};
let rules = matches.is_present("rules");
let rules_tags = match matches.values_of("rules-tags") {
Some(f) => f.map(String::from).collect(),
None => vec![],
};
let rules_include = match matches.values_of("rules-include") {
Some(f) => f.map(String::from).collect(),
None => vec![],
};
let rules_exclude = match matches.values_of("rules-exclude") {
Some(f) => f.map(String::from).collect(),
None => vec![],
};
let json = matches.is_present("json");
flags.subcommand = DenoSubcommand::Lint {
files,
rules,
rules_tags,
rules_include,
rules_exclude,
ignore,
json,
};
@ -2456,6 +2504,9 @@ mod tests {
PathBuf::from("script_2.ts")
],
rules: false,
rules_tags: vec![],
rules_include: vec![],
rules_exclude: vec![],
json: false,
ignore: vec![],
},
@ -2471,6 +2522,9 @@ mod tests {
subcommand: DenoSubcommand::Lint {
files: vec![],
rules: false,
rules_tags: vec![],
rules_include: vec![],
rules_exclude: vec![],
json: false,
ignore: vec![
PathBuf::from("script_1.ts"),
@ -2488,6 +2542,32 @@ mod tests {
subcommand: DenoSubcommand::Lint {
files: vec![],
rules: true,
rules_tags: vec![],
rules_include: vec![],
rules_exclude: vec![],
json: false,
ignore: vec![],
},
..Flags::default()
}
);
let r = flags_from_vec(svec![
"deno",
"lint",
"--rules-tags=",
"--rules-include=ban-untagged-todo,no-undef",
"--rules-exclude=no-const-assign"
]);
assert_eq!(
r.unwrap(),
Flags {
subcommand: DenoSubcommand::Lint {
files: vec![],
rules: false,
rules_tags: svec![""],
rules_include: svec!["ban-untagged-todo", "no-undef"],
rules_exclude: svec!["no-const-assign"],
json: false,
ignore: vec![],
},
@ -2502,12 +2582,40 @@ mod tests {
subcommand: DenoSubcommand::Lint {
files: vec![PathBuf::from("script_1.ts")],
rules: false,
rules_tags: vec![],
rules_include: vec![],
rules_exclude: vec![],
json: true,
ignore: vec![],
},
..Flags::default()
}
);
let r = flags_from_vec(svec![
"deno",
"lint",
"--config",
"Deno.jsonc",
"--json",
"script_1.ts"
]);
assert_eq!(
r.unwrap(),
Flags {
subcommand: DenoSubcommand::Lint {
files: vec![PathBuf::from("script_1.ts")],
rules: false,
rules_tags: vec![],
rules_include: vec![],
rules_exclude: vec![],
json: true,
ignore: vec![],
},
config_path: Some("Deno.jsonc".to_string()),
..Flags::default()
}
);
}
#[test]

View file

@ -392,7 +392,7 @@ impl Inner {
ConfigFile::read(path)?
};
let (value, maybe_ignored_options) =
config_file.as_compiler_options()?;
config_file.to_compiler_options()?;
tsconfig.merge(&value);
self.maybe_config_file = Some(config_file);
self.maybe_config_uri = Some(config_url);

View file

@ -489,10 +489,14 @@ async fn lsp_command() -> Result<(), AnyError> {
lsp::start().await
}
#[allow(clippy::too_many_arguments)]
async fn lint_command(
_flags: Flags,
flags: Flags,
files: Vec<PathBuf>,
list_rules: bool,
rules_tags: Vec<String>,
rules_include: Vec<String>,
rules_exclude: Vec<String>,
ignore: Vec<PathBuf>,
json: bool,
) -> Result<(), AnyError> {
@ -501,7 +505,24 @@ async fn lint_command(
return Ok(());
}
tools::lint::lint_files(files, ignore, json).await
let program_state = ProgramState::build(flags.clone()).await?;
let maybe_lint_config =
if let Some(config_file) = &program_state.maybe_config_file {
config_file.to_lint_config()?
} else {
None
};
tools::lint::lint_files(
maybe_lint_config,
rules_tags,
rules_include,
rules_exclude,
files,
ignore,
json,
)
.await
}
async fn cache_command(
@ -1183,9 +1204,22 @@ fn get_subcommand(
DenoSubcommand::Lint {
files,
rules,
rules_tags,
rules_include,
rules_exclude,
ignore,
json,
} => lint_command(flags, files, rules, ignore, json).boxed_local(),
} => lint_command(
flags,
files,
rules,
rules_tags,
rules_include,
rules_exclude,
ignore,
json,
)
.boxed_local(),
DenoSubcommand::Repl { eval } => run_repl(flags, eval).boxed_local(),
DenoSubcommand::Run { script } => run_command(flags, script).boxed_local(),
DenoSubcommand::Test {

View file

@ -24,59 +24,85 @@ fn ignore_unexplicit_files() {
}
itest!(all {
args: "lint --unstable lint/file1.js lint/file2.ts lint/ignored_file.ts",
args: "lint lint/file1.js lint/file2.ts lint/ignored_file.ts",
output: "lint/expected.out",
exit_code: 1,
});
itest!(quiet {
args: "lint --unstable --quiet lint/file1.js",
args: "lint --quiet lint/file1.js",
output: "lint/expected_quiet.out",
exit_code: 1,
});
itest!(json {
args:
"lint --unstable --json lint/file1.js lint/file2.ts lint/ignored_file.ts lint/malformed.js",
"lint --json lint/file1.js lint/file2.ts lint/ignored_file.ts lint/malformed.js",
output: "lint/expected_json.out",
exit_code: 1,
});
itest!(ignore {
args: "lint --unstable --ignore=lint/file1.js,lint/malformed.js lint/",
args:
"lint --ignore=lint/file1.js,lint/malformed.js,lint/lint_with_config/ lint/",
output: "lint/expected_ignore.out",
exit_code: 1,
});
itest!(glob {
args: "lint --unstable --ignore=lint/malformed.js lint/",
args: "lint --ignore=lint/malformed.js,lint/lint_with_config/ lint/",
output: "lint/expected_glob.out",
exit_code: 1,
});
itest!(stdin {
args: "lint --unstable -",
args: "lint -",
input: Some("let _a: any;"),
output: "lint/expected_from_stdin.out",
exit_code: 1,
});
itest!(stdin_json {
args: "lint --unstable --json -",
args: "lint --json -",
input: Some("let _a: any;"),
output: "lint/expected_from_stdin_json.out",
exit_code: 1,
});
itest!(rules {
args: "lint --unstable --rules",
args: "lint --rules",
output: "lint/expected_rules.out",
exit_code: 0,
});
// Make sure that the rules are printed if quiet option is enabled.
itest!(rules_quiet {
args: "lint --unstable --rules -q",
args: "lint --rules -q",
output: "lint/expected_rules.out",
exit_code: 0,
});
itest!(lint_with_config {
args: "lint --config lint/Deno.jsonc lint/lint_with_config/",
output: "lint/lint_with_config.out",
exit_code: 1,
});
// Check if CLI flags take precedence
itest!(lint_with_config_and_flags {
args: "lint --config lint/Deno.jsonc --ignore=lint/lint_with_config/a.ts",
output: "lint/lint_with_config_and_flags.out",
exit_code: 1,
});
itest!(lint_with_malformed_config {
args: "lint --config lint/Deno.malformed.jsonc",
output: "lint/lint_with_malformed_config.out",
exit_code: 1,
});
itest!(lint_with_malformed_config2 {
args: "lint --config lint/Deno.malformed2.jsonc",
output: "lint/lint_with_malformed_config2.out",
exit_code: 1,
});

12
cli/tests/testdata/lint/Deno.jsonc vendored Normal file
View file

@ -0,0 +1,12 @@
{
"lint": {
"files": {
"include": ["lint/lint_with_config/"],
"exclude": ["lint/lint_with_config/b.ts"]
},
"rules": {
"tags": ["recommended"],
"include": ["ban-untagged-todo"]
}
}
}

View file

@ -0,0 +1,13 @@
{
"lint": {
"files": {
"include": ["lint/lint_with_config/"],
"exclude": ["lint/lint_with_config/b.ts"]
},
"dont_know_this_field": {},
"rules": {
"tags": ["recommended"],
"include": ["ban-untagged-todo"]
}
}
}

View file

@ -0,0 +1,13 @@
{
"lint": {
"files": {
"include": ["lint/lint_with_config/"],
"exclude": ["lint/lint_with_config/b.ts"],
"dont_know_this_field": {}
},
"rules": {
"tags": ["recommended"],
"include": ["ban-untagged-todo"]
}
}
}

View file

@ -0,0 +1,18 @@
(ban-untagged-todo) TODO should be tagged with (@username) or (#issue)
// TODO: foo
^^^^^^^^^^^^
at [WILDCARD]a.ts:1:0
hint: Add a user tag or issue reference to the TODO comment, e.g. TODO(@djones), TODO(djones), TODO(#123)
help: for further information visit https://lint.deno.land/#ban-untagged-todo
(no-unused-vars) `add` is never used
function add(a: number, b: number): number {
^^^
at [WILDCARD]a.ts:2:9
hint: If this is intentional, prefix it with an underscore like `_add`
help: for further information visit https://lint.deno.land/#no-unused-vars
Found 2 problems
Checked 1 file

View file

@ -0,0 +1,4 @@
// TODO: foo
function add(a: number, b: number): number {
return a + b;
}

View file

@ -0,0 +1,4 @@
// TODO: this file should be ignored
function subtract(a: number, b: number): number {
return a - b;
}

View file

@ -0,0 +1,18 @@
(ban-untagged-todo) TODO should be tagged with (@username) or (#issue)
// TODO: this file should be ignored
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at [WILDCARD]b.ts:1:0
hint: Add a user tag or issue reference to the TODO comment, e.g. TODO(@djones), TODO(djones), TODO(#123)
help: for further information visit https://lint.deno.land/#ban-untagged-todo
(no-unused-vars) `subtract` is never used
function subtract(a: number, b: number): number {
^^^^^^^^
at [WILDCARD]b.ts:2:9
hint: If this is intentional, prefix it with an underscore like `_subtract`
help: for further information visit https://lint.deno.land/#no-unused-vars
Found 2 problems
Checked 1 file

View file

@ -0,0 +1,4 @@
error: Failed to parse "lint" configuration
Caused by:
unknown field `dont_know_this_field`, expected `rules` or `files`

View file

@ -0,0 +1,4 @@
error: Failed to parse "lint" configuration
Caused by:
unknown field `dont_know_this_field`, expected `include` or `exclude`

View file

@ -8,11 +8,12 @@
//! the same functions as ops available in JS runtime.
use crate::ast;
use crate::colors;
use crate::config_file::LintConfig;
use crate::fmt_errors;
use crate::fs_util::{collect_files, is_supported_ext};
use crate::media_type::MediaType;
use crate::tools::fmt::run_parallelized;
use deno_core::error::{generic_error, AnyError, JsStackFrame};
use deno_core::error::{anyhow, generic_error, AnyError, JsStackFrame};
use deno_core::serde_json;
use deno_lint::diagnostic::LintDiagnostic;
use deno_lint::linter::Linter;
@ -42,21 +43,60 @@ fn create_reporter(kind: LintReporterKind) -> Box<dyn LintReporter + Send> {
}
pub async fn lint_files(
maybe_lint_config: Option<LintConfig>,
rules_tags: Vec<String>,
rules_include: Vec<String>,
rules_exclude: Vec<String>,
args: Vec<PathBuf>,
ignore: Vec<PathBuf>,
json: bool,
) -> Result<(), AnyError> {
if args.len() == 1 && args[0].to_string_lossy() == "-" {
return lint_stdin(json);
return lint_stdin(
json,
maybe_lint_config.as_ref(),
rules_tags,
rules_include,
rules_exclude,
);
}
// Collect included and ignored files. CLI flags take precendence
// over config file, ie. if there's `files.ignore` in config file
// and `--ignore` CLI flag, only the flag value is taken into account.
let mut include_files = args;
let mut exclude_files = ignore;
if let Some(lint_config) = maybe_lint_config.as_ref() {
if include_files.is_empty() {
include_files = lint_config
.files
.include
.iter()
.map(PathBuf::from)
.collect::<Vec<PathBuf>>();
}
if exclude_files.is_empty() {
exclude_files = lint_config
.files
.exclude
.iter()
.map(PathBuf::from)
.collect::<Vec<PathBuf>>();
}
}
let target_files =
collect_files(&args, &ignore, is_supported_ext).and_then(|files| {
if files.is_empty() {
Err(generic_error("No target files found."))
} else {
Ok(files)
}
})?;
collect_files(&include_files, &exclude_files, is_supported_ext).and_then(
|files| {
if files.is_empty() {
Err(generic_error("No target files found."))
} else {
Ok(files)
}
},
)?;
debug!("Found {} files", target_files.len());
let target_files_len = target_files.len();
@ -69,11 +109,29 @@ pub async fn lint_files(
};
let reporter_lock = Arc::new(Mutex::new(create_reporter(reporter_kind)));
// Try to get configured rules. CLI flags take precendence
// over config file, ie. if there's `rules.include` in config file
// and `--rules-include` CLI flag, only the flag value is taken into account.
// TODO(bartlomieju): this is done multiple times for each file because
// Vec<Box<dyn LintRule>> is not clonable, this should be optimized.
get_configured_rules(
maybe_lint_config.as_ref(),
rules_tags.clone(),
rules_include.clone(),
rules_exclude.clone(),
)?;
run_parallelized(target_files, {
let reporter_lock = reporter_lock.clone();
let has_error = has_error.clone();
move |file_path| {
let r = lint_file(file_path.clone());
let r = lint_file(
file_path.clone(),
maybe_lint_config.as_ref(),
rules_tags.clone(),
rules_include.clone(),
rules_exclude.clone(),
);
let mut reporter = reporter_lock.lock().unwrap();
match r {
@ -144,13 +202,24 @@ pub fn create_linter(syntax: Syntax, rules: Vec<Box<dyn LintRule>>) -> Linter {
fn lint_file(
file_path: PathBuf,
maybe_lint_config: Option<&LintConfig>,
rules_tags: Vec<String>,
rules_include: Vec<String>,
rules_exclude: Vec<String>,
) -> Result<(Vec<LintDiagnostic>, String), AnyError> {
let file_name = file_path.to_string_lossy().to_string();
let source_code = fs::read_to_string(&file_path)?;
let media_type = MediaType::from(&file_path);
let syntax = ast::get_syntax(&media_type);
let lint_rules = rules::get_recommended_rules();
// Obtaining rules from config is infallible at this point.
let lint_rules = get_configured_rules(
maybe_lint_config,
rules_tags,
rules_include,
rules_exclude,
)
.unwrap();
let linter = create_linter(syntax, lint_rules);
let (_, file_diagnostics) = linter.lint(file_name, source_code.clone())?;
@ -161,7 +230,13 @@ fn lint_file(
/// Lint stdin and write result to stdout.
/// Treats input as TypeScript.
/// Compatible with `--json` flag.
fn lint_stdin(json: bool) -> Result<(), AnyError> {
fn lint_stdin(
json: bool,
maybe_lint_config: Option<&LintConfig>,
rules_tags: Vec<String>,
rules_include: Vec<String>,
rules_exclude: Vec<String>,
) -> Result<(), AnyError> {
let mut source = String::new();
if stdin().read_to_string(&mut source).is_err() {
return Err(generic_error("Failed to read from stdin"));
@ -173,7 +248,12 @@ fn lint_stdin(json: bool) -> Result<(), AnyError> {
LintReporterKind::Pretty
};
let mut reporter = create_reporter(reporter_kind);
let lint_rules = rules::get_recommended_rules();
let lint_rules = get_configured_rules(
maybe_lint_config,
rules_tags,
rules_include,
rules_exclude,
)?;
let syntax = ast::get_syntax(&MediaType::TypeScript);
let linter = create_linter(syntax, lint_rules);
let mut has_error = false;
@ -385,3 +465,59 @@ fn sort_diagnostics(diagnostics: &mut Vec<LintDiagnostic>) {
}
});
}
fn get_configured_rules(
maybe_lint_config: Option<&LintConfig>,
rules_tags: Vec<String>,
rules_include: Vec<String>,
rules_exclude: Vec<String>,
) -> Result<Vec<Box<dyn LintRule>>, AnyError> {
if maybe_lint_config.is_none()
&& rules_tags.is_empty()
&& rules_include.is_empty()
&& rules_exclude.is_empty()
{
return Ok(rules::get_recommended_rules());
}
let (config_file_tags, config_file_include, config_file_exclude) =
if let Some(lint_config) = maybe_lint_config.as_ref() {
(
lint_config.rules.tags.clone(),
lint_config.rules.include.clone(),
lint_config.rules.exclude.clone(),
)
} else {
(None, None, None)
};
let maybe_configured_include = if !rules_include.is_empty() {
Some(rules_include)
} else {
config_file_include
};
let maybe_configured_exclude = if !rules_exclude.is_empty() {
Some(rules_exclude)
} else {
config_file_exclude
};
let configured_tags = if !rules_tags.is_empty() {
rules_tags
} else {
config_file_tags.unwrap_or_else(Vec::new)
};
let configured_rules = rules::get_filtered_rules(
Some(configured_tags),
maybe_configured_exclude,
maybe_configured_include,
);
if configured_rules.is_empty() {
anyhow!("No rules have been configured");
}
Ok(configured_rules)
}