Skip to content

Commit

Permalink
feat: add --no-check=remote flag (#12766)
Browse files Browse the repository at this point in the history
Closes #11970
  • Loading branch information
kitsonk authored Nov 29, 2021
1 parent f3b7435 commit 18a63dd
Show file tree
Hide file tree
Showing 13 changed files with 133 additions and 21 deletions.
10 changes: 10 additions & 0 deletions cli/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,16 @@ impl Diagnostics {
}));
}

/// Return a set of diagnostics where only the values where the predicate
/// returns `true` are included.
pub fn filter<P>(&self, predicate: P) -> Self
where
P: FnMut(&Diagnostic) -> bool,
{
let diagnostics = self.0.clone().into_iter().filter(predicate).collect();
Self(diagnostics)
}

pub fn is_empty(&self) -> bool {
self.0.is_empty()
}
Expand Down
20 changes: 18 additions & 2 deletions cli/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::config_file::ConfigFile;
use crate::config_file::IgnoredCompilerOptions;
use crate::config_file::TsConfig;
use crate::diagnostics::Diagnostics;
use crate::flags;
use crate::tsc;
use crate::version;

Expand Down Expand Up @@ -293,6 +294,9 @@ fn is_emittable(media_type: &MediaType, include_js: bool) -> bool {
/// Options for performing a check of a module graph. Note that the decision to
/// emit or not is determined by the `ts_config` settings.
pub(crate) struct CheckOptions {
/// The check flag from the option which can effect the filtering of
/// diagnostics in the emit result.
pub check: flags::CheckFlag,
/// Set the debug flag on the TypeScript type checker.
pub debug: bool,
/// If true, any files emitted will be cached, even if there are diagnostics
Expand Down Expand Up @@ -357,9 +361,21 @@ pub(crate) fn check_and_maybe_emit(
root_names,
})?;

let diagnostics = if options.check == flags::CheckFlag::Local {
response.diagnostics.filter(|d| {
if let Some(file_name) = &d.file_name {
!file_name.starts_with("http")
} else {
true
}
})
} else {
response.diagnostics
};

// sometimes we want to emit when there are diagnostics, and sometimes we
// don't. tsc will always return an emit if there are diagnostics
if (response.diagnostics.is_empty() || options.emit_with_diagnostics)
if (diagnostics.is_empty() || options.emit_with_diagnostics)
&& !response.emitted_files.is_empty()
{
if let Some(info) = &response.maybe_tsbuildinfo {
Expand Down Expand Up @@ -414,7 +430,7 @@ pub(crate) fn check_and_maybe_emit(
}

Ok(CheckEmitResult {
diagnostics: response.diagnostics,
diagnostics,
stats: response.stats,
})
}
Expand Down
69 changes: 60 additions & 9 deletions cli/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,24 @@ impl Default for DenoSubcommand {
}
}

#[derive(Debug, Clone, PartialEq)]
pub enum CheckFlag {
/// Type check all modules. The default value.
All,
/// Skip type checking of all modules. Represents `--no-check` on the command
/// line.
None,
/// Only type check local modules. Represents `--no-check=remote` on the
/// command line.
Local,
}

impl Default for CheckFlag {
fn default() -> Self {
Self::All
}
}

#[derive(Clone, Debug, PartialEq, Default)]
pub struct Flags {
/// Vector of CLI arguments - these are user script arguments, all Deno
Expand All @@ -209,6 +227,7 @@ pub struct Flags {
/// the language server is configured with an explicit cache option.
pub cache_path: Option<PathBuf>,
pub cached_only: bool,
pub check: CheckFlag,
pub config_path: Option<String>,
pub coverage_dir: Option<String>,
pub enable_testing_features: bool,
Expand All @@ -220,7 +239,6 @@ pub struct Flags {
pub lock_write: bool,
pub lock: Option<PathBuf>,
pub log_level: Option<Level>,
pub no_check: bool,
pub no_remote: bool,
/// If true, a list of Node built-in modules will be injected into
/// the import map.
Expand Down Expand Up @@ -1643,8 +1661,17 @@ Only local files from entry point module graph are watched.",

fn no_check_arg<'a, 'b>() -> Arg<'a, 'b> {
Arg::with_name("no-check")
.takes_value(true)
.require_equals(true)
.min_values(0)
.value_name("NO_CHECK_TYPE")
.long("no-check")
.help("Skip type checking modules")
.long_help(
"Skip type checking of modules.
If the value of '--no-check=remote' is supplied, diagnostic errors from remote
modules will be ignored.",
)
}

fn script_arg<'a, 'b>() -> Arg<'a, 'b> {
Expand Down Expand Up @@ -2334,8 +2361,16 @@ fn compat_arg_parse(flags: &mut Flags, matches: &ArgMatches) {
}

fn no_check_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
if matches.is_present("no-check") {
flags.no_check = true;
if let Some(cache_type) = matches.value_of("no-check") {
match cache_type {
"remote" => flags.check = CheckFlag::Local,
_ => debug!(
"invalid value for 'no-check' of '{}' using default",
cache_type
),
}
} else if matches.is_present("no-check") {
flags.check = CheckFlag::None;
}
}

Expand Down Expand Up @@ -3131,7 +3166,7 @@ mod tests {
import_map_path: Some("import_map.json".to_string()),
no_remote: true,
config_path: Some("tsconfig.json".to_string()),
no_check: true,
check: CheckFlag::None,
reload: true,
lock: Some(PathBuf::from("lock.json")),
lock_write: true,
Expand Down Expand Up @@ -3216,7 +3251,7 @@ mod tests {
import_map_path: Some("import_map.json".to_string()),
no_remote: true,
config_path: Some("tsconfig.json".to_string()),
no_check: true,
check: CheckFlag::None,
reload: true,
lock: Some(PathBuf::from("lock.json")),
lock_write: true,
Expand Down Expand Up @@ -3483,7 +3518,7 @@ mod tests {
source_file: "script.ts".to_string(),
out_file: None,
}),
no_check: true,
check: CheckFlag::None,
..Flags::default()
}
);
Expand Down Expand Up @@ -3682,7 +3717,7 @@ mod tests {
import_map_path: Some("import_map.json".to_string()),
no_remote: true,
config_path: Some("tsconfig.json".to_string()),
no_check: true,
check: CheckFlag::None,
reload: true,
lock: Some(PathBuf::from("lock.json")),
lock_write: true,
Expand Down Expand Up @@ -3833,7 +3868,23 @@ mod tests {
subcommand: DenoSubcommand::Run(RunFlags {
script: "script.ts".to_string(),
}),
no_check: true,
check: CheckFlag::None,
..Flags::default()
}
);
}

#[test]
fn no_check_remote() {
let r =
flags_from_vec(svec!["deno", "run", "--no-check=remote", "script.ts"]);
assert_eq!(
r.unwrap(),
Flags {
subcommand: DenoSubcommand::Run(RunFlags {
script: "script.ts".to_string(),
}),
check: CheckFlag::Local,
..Flags::default()
}
);
Expand Down Expand Up @@ -4406,7 +4457,7 @@ mod tests {
import_map_path: Some("import_map.json".to_string()),
no_remote: true,
config_path: Some("tsconfig.json".to_string()),
no_check: true,
check: CheckFlag::None,
reload: true,
lock: Some(PathBuf::from("lock.json")),
lock_write: true,
Expand Down
6 changes: 4 additions & 2 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use crate::file_fetcher::File;
use crate::file_watcher::ResolutionResult;
use crate::flags::BundleFlags;
use crate::flags::CacheFlags;
use crate::flags::CheckFlag;
use crate::flags::CompileFlags;
use crate::flags::CompletionsFlags;
use crate::flags::CoverageFlags;
Expand Down Expand Up @@ -706,7 +707,7 @@ async fn create_graph_and_maybe_check(
// locker.
emit::lock(graph.as_ref());

if !ps.flags.no_check {
if ps.flags.check != CheckFlag::None {
graph.valid_types_only().map_err(emit::GraphError::from)?;
let lib = if ps.flags.unstable {
emit::TypeLib::UnstableDenoWindow
Expand All @@ -731,6 +732,7 @@ async fn create_graph_and_maybe_check(
graph.clone(),
&mut cache,
emit::CheckOptions {
check: ps.flags.check.clone(),
debug,
emit_with_diagnostics: false,
maybe_config_specifier,
Expand Down Expand Up @@ -759,7 +761,7 @@ fn bundle_module_graph(
ps.maybe_config_file.as_ref(),
None,
)?;
if flags.no_check {
if flags.check == CheckFlag::None {
if let Some(ignored_options) = maybe_ignored_options {
eprintln!("{}", ignored_options);
}
Expand Down
3 changes: 3 additions & 0 deletions cli/ops/runtime_compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::config_file::IgnoredCompilerOptions;
use crate::diagnostics::Diagnostics;
use crate::emit;
use crate::errors::get_error_class_name;
use crate::flags;
use crate::proc_state::ProcState;
use crate::resolver::ImportMapResolver;
use crate::resolver::JsxResolver;
Expand Down Expand Up @@ -248,6 +249,7 @@ async fn op_emit(
graph.clone(),
cache.as_mut_cacher(),
emit::CheckOptions {
check: flags::CheckFlag::All,
debug,
emit_with_diagnostics: true,
maybe_config_specifier: None,
Expand All @@ -268,6 +270,7 @@ async fn op_emit(
graph.clone(),
cache.as_mut_cacher(),
emit::CheckOptions {
check: flags::CheckFlag::All,
debug,
emit_with_diagnostics: true,
maybe_config_specifier: None,
Expand Down
9 changes: 5 additions & 4 deletions cli/proc_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ impl ProcState {
};
if !reload_on_watch {
let graph_data = self.graph_data.lock();
if self.flags.no_check
if self.flags.check == flags::CheckFlag::None
|| roots.iter().all(|root| {
graph_data
.checked_libs_map
Expand Down Expand Up @@ -452,7 +452,7 @@ impl ProcState {
graph_data.modules.keys().cloned().collect()
};

let config_type = if self.flags.no_check {
let config_type = if self.flags.check == flags::CheckFlag::None {
emit::ConfigType::Emit
} else {
emit::ConfigType::Check {
Expand Down Expand Up @@ -483,7 +483,7 @@ impl ProcState {
if let Some(ignored_options) = maybe_ignored_options {
log::warn!("{}", ignored_options);
}
let emit_result = if self.flags.no_check {
let emit_result = if self.flags.check == flags::CheckFlag::None {
let options = emit::EmitOptions {
ts_config,
reload_exclusions,
Expand All @@ -502,6 +502,7 @@ impl ProcState {
.as_ref()
.map(|cf| cf.specifier.clone());
let options = emit::CheckOptions {
check: self.flags.check.clone(),
debug: self.flags.log_level == Some(log::Level::Debug),
emit_with_diagnostics: false,
maybe_config_specifier,
Expand Down Expand Up @@ -599,7 +600,7 @@ impl ProcState {
graph_data.check_if_prepared(&roots).unwrap()?;
type_check_result?;

if !self.flags.no_check {
if self.flags.check != flags::CheckFlag::None {
for specifier in specifiers.keys() {
let checked_libs = graph_data
.checked_libs_map
Expand Down
13 changes: 13 additions & 0 deletions cli/tests/integration/run_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,19 @@ itest!(no_check_decorators {
output: "no_check_decorators.ts.out",
});

itest!(check_remote {
args: "run --quiet --reload no_check_remote.ts",
output: "no_check_remote.ts.disabled.out",
exit_code: 1,
http_server: true,
});

itest!(no_check_remote {
args: "run --quiet --reload --no-check=remote no_check_remote.ts",
output: "no_check_remote.ts.enabled.out",
http_server: true,
});

itest!(runtime_decorators {
args: "run --quiet --reload --no-check runtime_decorators.ts",
output: "runtime_decorators.ts.out",
Expand Down
3 changes: 3 additions & 0 deletions cli/tests/testdata/no_check_remote.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import * as a from "http://localhost:4545/subdir/type_error.ts";

console.log(a.a);
4 changes: 4 additions & 0 deletions cli/tests/testdata/no_check_remote.ts.disabled.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
error: TS2322 [ERROR]: Type '12' is not assignable to type '"a"'.
export const a: "a" = 12;
^
at http://localhost:4545/subdir/type_error.ts:1:14
1 change: 1 addition & 0 deletions cli/tests/testdata/no_check_remote.ts.enabled.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
12
1 change: 1 addition & 0 deletions cli/tests/testdata/subdir/type_error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const a: "a" = 12;
12 changes: 9 additions & 3 deletions cli/tools/installer.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.

use crate::flags::CheckFlag;
use crate::flags::Flags;
use crate::fs_util::canonicalize_path;
use deno_core::error::generic_error;
Expand Down Expand Up @@ -267,8 +269,12 @@ pub fn install(
}
}

if flags.no_check {
executable_args.push("--no-check".to_string());
// we should avoid a default branch here to ensure we continue to cover any
// changes to this flag.
match flags.check {
CheckFlag::All => (),
CheckFlag::None => executable_args.push("--no-check".to_string()),
CheckFlag::Local => executable_args.push("--no-check=remote".to_string()),
}

if flags.unstable {
Expand Down Expand Up @@ -687,7 +693,7 @@ mod tests {
Flags {
allow_net: Some(vec![]),
allow_read: Some(vec![]),
no_check: true,
check: CheckFlag::None,
log_level: Some(Level::Error),
..Flags::default()
},
Expand Down
3 changes: 2 additions & 1 deletion cli/tools/standalone.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.

use crate::deno_dir::DenoDir;
use crate::flags::CheckFlag;
use crate::flags::DenoSubcommand;
use crate::flags::Flags;
use crate::flags::RunFlags;
Expand Down Expand Up @@ -226,7 +227,7 @@ pub fn compile_to_runtime_flags(
lock_write: false,
lock: None,
log_level: flags.log_level,
no_check: false,
check: CheckFlag::All,
compat: flags.compat,
unsafely_ignore_certificate_errors: flags
.unsafely_ignore_certificate_errors,
Expand Down

0 comments on commit 18a63dd

Please sign in to comment.