Skip to content

Commit

Permalink
fix: better cli file argument handling (#72)
Browse files Browse the repository at this point in the history
Fixes a bug where things would fall apart when arguments were provided.

This now takes the CLI arguments and merges them in with the config in
the different workspace members.
  • Loading branch information
dsherret authored Jul 8, 2024
1 parent 9839ba1 commit 0983534
Show file tree
Hide file tree
Showing 9 changed files with 1,082 additions and 209 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 rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[toolchain]
channel = "1.71.0"
channel = "1.78.0"
components = ["rustfmt", "clippy"]
37 changes: 24 additions & 13 deletions src/deno_json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl SerializedLintConfig {
let files = SerializedFilesConfig { include, exclude };

Ok(LintConfig {
rules: self.rules,
options: LintOptionsConfig { rules: self.rules },
files: choose_files(files, self.deprecated_files)
.into_resolved(config_file_specifier)?,
})
Expand All @@ -178,9 +178,14 @@ pub struct WorkspaceLintConfig {
pub report: Option<String>,
}

#[derive(Clone, Debug, Default, Hash, PartialEq)]
pub struct LintOptionsConfig {
pub rules: LintRulesConfig,
}

#[derive(Clone, Debug, Hash, PartialEq)]
pub struct LintConfig {
pub rules: LintRulesConfig,
pub options: LintOptionsConfig,
pub files: FilePatterns,
}

Expand Down Expand Up @@ -1177,7 +1182,7 @@ impl ConfigFile {
.context("Invalid lint config.")
}
None => Ok(LintConfig {
rules: Default::default(),
options: Default::default(),
files: self.to_exclude_files_config()?,
}),
}
Expand Down Expand Up @@ -1257,7 +1262,8 @@ impl ConfigFile {
/// If the configuration file contains "extra" modules (like TypeScript
/// `"types"`) options, return them as imports to be added to a module graph.
pub fn to_maybe_imports(&self) -> Result<Vec<(Url, Vec<String>)>, AnyError> {
let Some(compiler_options_value) = self.json.compiler_options.as_ref() else {
let Some(compiler_options_value) = self.json.compiler_options.as_ref()
else {
return Ok(Vec::new());
};
let Some(types) = compiler_options_value.get("types") else {
Expand All @@ -1277,13 +1283,16 @@ impl ConfigFile {
pub fn to_maybe_jsx_import_source_config(
&self,
) -> Result<Option<JsxImportSourceConfig>, AnyError> {
let Some(compiler_options_value) = self.json.compiler_options.as_ref() else {
let Some(compiler_options_value) = self.json.compiler_options.as_ref()
else {
return Ok(None);
};
let Some(compiler_options) =
serde_json::from_value::<CompilerOptions>(compiler_options_value.clone()).ok() else {
return Ok(None);
};
serde_json::from_value::<CompilerOptions>(compiler_options_value.clone())
.ok()
else {
return Ok(None);
};
let module = match compiler_options.jsx.as_deref() {
Some("react-jsx") => "jsx-runtime".to_string(),
Some("react-jsxdev") => "jsx-dev-runtime".to_string(),
Expand Down Expand Up @@ -1611,11 +1620,13 @@ mod tests {
PathBuf::from("/deno/src/testdata/")
)]),
},
rules: LintRulesConfig {
include: Some(vec!["ban-untagged-todo".to_string()]),
exclude: None,
tags: Some(vec!["recommended".to_string()]),
},
options: LintOptionsConfig {
rules: LintRulesConfig {
include: Some(vec!["ban-untagged-todo".to_string()]),
exclude: None,
tags: Some(vec!["recommended".to_string()]),
},
}
}
);
assert_eq!(
Expand Down
8 changes: 4 additions & 4 deletions src/glob/collector.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright 2018-2024 the Deno authors. MIT license.

use std::collections::HashSet;
use std::collections::VecDeque;
use std::path::Path;
use std::path::PathBuf;
Expand All @@ -11,7 +12,6 @@ use crate::glob::FilePatternsMatch;
use crate::glob::PathKind;
use crate::glob::PathOrPattern;
use crate::util::normalize_path;
use crate::util::CheckedSet;

use super::FilePatterns;

Expand Down Expand Up @@ -118,7 +118,7 @@ impl<TFilter: Fn(WalkEntry) -> bool> FileCollector<TFilter> {
None
};
let mut target_files = Vec::new();
let mut visited_paths: CheckedSet<Path> = CheckedSet::default();
let mut visited_paths: HashSet<PathBuf> = HashSet::default();
let file_patterns_by_base = file_patterns.split_by_base();
for file_patterns in file_patterns_by_base {
let specified_path = normalize_path(&file_patterns.base);
Expand Down Expand Up @@ -147,14 +147,14 @@ impl<TFilter: Fn(WalkEntry) -> bool> FileCollector<TFilter> {
let opt_out_ignore = specified_path == path;
let should_ignore_dir =
!opt_out_ignore && self.is_ignored_dir(&path);
if !should_ignore_dir && visited_paths.insert(&path) {
if !should_ignore_dir && visited_paths.insert(path.clone()) {
pending_dirs.push_back(path);
}
} else if (self.file_filter)(WalkEntry {
path: &path,
metadata,
patterns: &file_patterns,
}) && visited_paths.insert(&path)
}) && visited_paths.insert(path.clone())
{
target_files.push(path);
}
Expand Down
21 changes: 19 additions & 2 deletions src/glob/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,17 @@ impl FilePatterns {
// Sort by the longest base path first. This ensures that we visit opted into
// nested directories first before visiting the parent directory. The directory
// traverser will handle not going into directories it's already been in.
result
.sort_by(|a, b| b.base.as_os_str().len().cmp(&a.base.as_os_str().len()));
result.sort_by(|a, b| {
// try looking at the parents first so that files in the same
// folder are kept in the same order that they're provided
let (a, b) =
if let (Some(a), Some(b)) = (a.base.parent(), b.base.parent()) {
(a, b)
} else {
(a.base.as_path(), b.base.as_path())
};
b.as_os_str().len().cmp(&a.as_os_str().len())
});

result
}
Expand Down Expand Up @@ -387,6 +396,10 @@ impl PathOrPatternSet {
&self.0
}

pub fn inner_mut(&mut self) -> &mut Vec<PathOrPattern> {
&mut self.0
}

pub fn into_path_or_patterns(self) -> Vec<PathOrPattern> {
self.0
}
Expand Down Expand Up @@ -426,6 +439,10 @@ impl PathOrPatternSet {
result
}

pub fn push(&mut self, item: PathOrPattern) {
self.0.push(item);
}

pub fn append(&mut self, items: impl Iterator<Item = PathOrPattern>) {
self.0.extend(items)
}
Expand Down
37 changes: 0 additions & 37 deletions src/util.rs
Original file line number Diff line number Diff line change
@@ -1,48 +1,11 @@
// Copyright 2018-2024 the Deno authors. MIT license.

use std::marker::PhantomData;
use std::path::Component;
use std::path::Path;
use std::path::PathBuf;
use thiserror::Error;
use url::Url;

#[derive(Debug)]
pub struct CheckedSet<T: std::hash::Hash + ?Sized> {
_kind: PhantomData<T>,
checked: std::collections::HashSet<u64>,
}

impl<T: std::hash::Hash + ?Sized> Default for CheckedSet<T> {
fn default() -> Self {
Self {
_kind: Default::default(),
checked: Default::default(),
}
}
}

impl<T: std::hash::Hash + ?Sized> CheckedSet<T> {
pub fn with_capacity(capacity: usize) -> Self {
Self {
_kind: PhantomData,
checked: std::collections::HashSet::with_capacity(capacity),
}
}

pub fn insert(&mut self, value: &T) -> bool {
self.checked.insert(self.get_hash(value))
}

fn get_hash(&self, value: &T) -> u64 {
use std::collections::hash_map::DefaultHasher;
use std::hash::Hasher;
let mut hasher = DefaultHasher::new();
value.hash(&mut hasher);
hasher.finish()
}
}

pub fn is_skippable_io_error(e: &std::io::Error) -> bool {
use std::io::ErrorKind::*;
match e.kind() {
Expand Down
10 changes: 5 additions & 5 deletions src/workspace/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::Path;
use std::path::PathBuf;

Expand All @@ -16,7 +17,6 @@ use crate::util::is_skippable_io_error;
use crate::util::normalize_path;
use crate::util::specifier_parent;
use crate::util::specifier_to_file_path;
use crate::util::CheckedSet;
use crate::ConfigFile;
use crate::ConfigFileRc;

Expand Down Expand Up @@ -149,15 +149,15 @@ pub fn discover_workspace_config_files(
opts: &WorkspaceDiscoverOptions,
) -> Result<ConfigFileDiscovery, WorkspaceDiscoverError> {
match start {
WorkspaceDiscoverStart::Dirs(dirs) => match dirs.len() {
WorkspaceDiscoverStart::Paths(dirs) => match dirs.len() {
0 => Ok(ConfigFileDiscovery::None),
1 => {
let dir = &dirs[0];
let start = DirOrConfigFile::Dir(dir);
discover_workspace_config_files_for_single_dir(start, opts, None)
}
_ => {
let mut checked = CheckedSet::default();
let mut checked = HashSet::default();
let mut final_workspace = ConfigFileDiscovery::None;
for dir in dirs {
let workspace = discover_workspace_config_files_for_single_dir(
Expand Down Expand Up @@ -201,7 +201,7 @@ enum DirOrConfigFile<'a> {
fn discover_workspace_config_files_for_single_dir(
start: DirOrConfigFile,
opts: &WorkspaceDiscoverOptions,
mut checked: Option<&mut CheckedSet<Path>>,
mut checked: Option<&mut HashSet<PathBuf>>,
) -> Result<ConfigFileDiscovery, WorkspaceDiscoverError> {
fn strip_up_to_node_modules(path: &Path) -> PathBuf {
path
Expand Down Expand Up @@ -288,7 +288,7 @@ fn discover_workspace_config_files_for_single_dir(
let start_dir = start_dir.map(strip_up_to_node_modules);
for current_dir in start_dir.iter().flat_map(|p| p.ancestors()) {
if let Some(checked) = checked.as_mut() {
if !checked.insert(current_dir) {
if !checked.insert(current_dir.to_path_buf()) {
// already visited here, so exit
return Ok(ConfigFileDiscovery::None);
}
Expand Down
Loading

0 comments on commit 0983534

Please sign in to comment.