feat: subcommands type-check only local files by default (#14623)

This commit changes default mode of type-checking to "local" 
and adds "--check" flag to following subcommands:
- deno bench
- deno bundle
- deno cache
- deno compile
- deno eval
- deno install
- deno test
This commit is contained in:
Bartek Iwańczuk 2022-05-17 23:53:42 +02:00 committed by GitHub
parent 330c820ae8
commit 9a85a95c43
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
35 changed files with 339 additions and 51 deletions

View File

@ -246,7 +246,9 @@ pub enum TypeCheckMode {
impl Default for TypeCheckMode {
fn default() -> Self {
Self::All
// TODO(bartlomieju): in v1.22 we switched to `Local` instead of `All` and
// in v1.23 we will switch to `None` by default.
Self::Local
}
}
@ -254,19 +256,20 @@ impl Default for TypeCheckMode {
// in 1.23)
#[derive(Debug, Clone, PartialEq)]
pub enum FutureTypeCheckMode {
/// Type check all modules. The default value.
/// Type check all modules. Represents `--check=all` on the command line.
All,
/// Skip type checking of all modules. Represents `--no-check` on the command
/// line.
/// Skip type checking of all modules. The default value.
None,
/// Only type check local modules. Represents `--no-check=remote` on the
/// Only type check local modules. Represents `--check` on the
/// command line.
Local,
}
impl Default for FutureTypeCheckMode {
fn default() -> Self {
Self::None
// TODO(bartlomieju): in v1.22 we switched to `Local` instead of `All` and
// in v1.23 we will switch to `None` by default.
Self::Local
}
}
@ -306,10 +309,12 @@ pub struct Flags {
pub cache_path: Option<PathBuf>,
pub cached_only: bool,
pub type_check_mode: TypeCheckMode,
// TODO(bartlomieju): should be removed in favor of `check`
// once type checking is skipped by default
pub future_type_check_mode: FutureTypeCheckMode,
// TODO(bartlomieju): to be removed in v1.23.
pub has_no_check_flag: bool,
// TODO(bartlomieju): to be removed in v1.23.
pub has_check_flag: bool,
// TODO(bartlomieju): to be removed in v1.23.
pub future_type_check_mode: FutureTypeCheckMode,
pub config_flag: ConfigFlag,
pub coverage_dir: Option<String>,
pub enable_testing_features: bool,
@ -759,7 +764,7 @@ Future runs of this module will trigger no downloads or compilation unless \
}
fn check_subcommand<'a>() -> Command<'a> {
compile_args_without_no_check(Command::new("check"))
compile_args_without_check_args(Command::new("check"))
.arg(
Arg::new("remote")
.long("remote")
@ -1427,7 +1432,6 @@ fn run_subcommand<'a>() -> Command<'a> {
.conflicts_with("inspect-brk"),
)
.arg(no_clear_screen_arg())
.arg(check_arg())
.trailing_var_arg(true)
.arg(script_arg().required(true))
.about("Run a JavaScript or TypeScript program")
@ -1717,13 +1721,14 @@ fn compile_args(app: Command) -> Command {
.arg(no_remote_arg())
.args(config_args())
.arg(no_check_arg())
.arg(check_arg())
.arg(reload_arg())
.arg(lock_arg())
.arg(lock_write_arg())
.arg(ca_file_arg())
}
fn compile_args_without_no_check(app: Command) -> Command {
fn compile_args_without_check_args(app: Command) -> Command {
app
.arg(import_map_arg())
.arg(no_remote_arg())
@ -2942,6 +2947,7 @@ fn compat_arg_parse(flags: &mut Flags, matches: &ArgMatches) {
}
fn no_check_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
flags.has_no_check_flag = matches.is_present("no-check");
if let Some(cache_type) = matches.value_of("no-check") {
match cache_type {
"remote" => flags.type_check_mode = TypeCheckMode::Local,
@ -3938,6 +3944,7 @@ mod tests {
import_map_path: Some("import_map.json".to_string()),
no_remote: true,
config_flag: ConfigFlag::Path("tsconfig.json".to_owned()),
has_no_check_flag: true,
type_check_mode: TypeCheckMode::None,
reload: true,
lock: Some(PathBuf::from("lock.json")),
@ -4030,6 +4037,7 @@ mod tests {
no_remote: true,
config_flag: ConfigFlag::Path("tsconfig.json".to_owned()),
type_check_mode: TypeCheckMode::None,
has_no_check_flag: true,
reload: true,
lock: Some(PathBuf::from("lock.json")),
lock_write: true,
@ -4330,6 +4338,7 @@ mod tests {
source_file: "script.ts".to_string(),
out_file: None,
}),
has_no_check_flag: true,
type_check_mode: TypeCheckMode::None,
..Flags::default()
}
@ -4552,6 +4561,7 @@ mod tests {
import_map_path: Some("import_map.json".to_string()),
no_remote: true,
config_flag: ConfigFlag::Path("tsconfig.json".to_owned()),
has_no_check_flag: true,
type_check_mode: TypeCheckMode::None,
reload: true,
lock: Some(PathBuf::from("lock.json")),
@ -4703,6 +4713,7 @@ mod tests {
subcommand: DenoSubcommand::Run(RunFlags {
script: "script.ts".to_string(),
}),
has_no_check_flag: true,
type_check_mode: TypeCheckMode::None,
..Flags::default()
}
@ -4719,6 +4730,7 @@ mod tests {
subcommand: DenoSubcommand::Run(RunFlags {
script: "script.ts".to_string(),
}),
has_no_check_flag: true,
type_check_mode: TypeCheckMode::Local,
..Flags::default()
}
@ -5397,6 +5409,7 @@ mod tests {
import_map_path: Some("import_map.json".to_string()),
no_remote: true,
config_flag: ConfigFlag::Path("tsconfig.json".to_owned()),
has_no_check_flag: true,
type_check_mode: TypeCheckMode::None,
reload: true,
lock: Some(PathBuf::from("lock.json")),
@ -5762,8 +5775,8 @@ mod tests {
subcommand: DenoSubcommand::Run(RunFlags {
script: "script.ts".to_string(),
}),
future_type_check_mode: FutureTypeCheckMode::Local,
has_check_flag: true,
future_type_check_mode: FutureTypeCheckMode::Local,
..Flags::default()
}
);
@ -5775,8 +5788,8 @@ mod tests {
subcommand: DenoSubcommand::Run(RunFlags {
script: "script.ts".to_string(),
}),
future_type_check_mode: FutureTypeCheckMode::All,
has_check_flag: true,
future_type_check_mode: FutureTypeCheckMode::All,
..Flags::default()
}
);
@ -5788,8 +5801,8 @@ mod tests {
subcommand: DenoSubcommand::Run(RunFlags {
script: "script.ts".to_string(),
}),
future_type_check_mode: FutureTypeCheckMode::None,
has_check_flag: true,
future_type_check_mode: FutureTypeCheckMode::Local,
..Flags::default()
}
);

View File

@ -1531,18 +1531,23 @@ pub fn main() {
logger::init(flags.log_level);
// TODO(bartlomieju): remove once type checking is skipped by default (probably
// in 1.23).
// If this env var is set we're gonna override default behavior of type checking
// and use behavior defined by the `--check` flag.
// TODO(bartlomieju): v1.22 is a "pivot version" in terms of default
// type checking mode. We're opting into type checking only local
// files by default and in v1.23 we're not gonna type check at all by default.
// So right now, we're still allowing to use `--no-check` flag and if it is
// present, we opt into the "old" behavior. Additionally, if
// "DENO_FUTURE_CHECK" env var is present we're switching to the new behavior
// of skipping type checking completely if no `--check` flag is present.
let future_check_env_var = env::var("DENO_FUTURE_CHECK").ok();
if let Some(env_var) = future_check_env_var {
if env_var == "1" {
flags.type_check_mode = match &flags.future_type_check_mode {
FutureTypeCheckMode::None => TypeCheckMode::None,
FutureTypeCheckMode::All => TypeCheckMode::All,
FutureTypeCheckMode::Local => TypeCheckMode::Local,
}
if env_var == "1" && !flags.has_check_flag {
flags.type_check_mode = TypeCheckMode::None;
}
} else if !flags.has_no_check_flag {
flags.type_check_mode = match &flags.future_type_check_mode {
FutureTypeCheckMode::None => TypeCheckMode::None,
FutureTypeCheckMode::All => TypeCheckMode::All,
FutureTypeCheckMode::Local => TypeCheckMode::Local,
}
}

View File

@ -155,6 +155,19 @@ itest!(no_prompt_with_denied_perms {
output: "bench/no_prompt_with_denied_perms.out",
});
itest!(check_local_by_default {
args: "bench --quiet --unstable bench/check_local_by_default.ts",
output: "bench/check_local_by_default.out",
http_server: true,
});
itest!(check_local_by_default2 {
args: "bench --quiet --unstable bench/check_local_by_default2.ts",
output: "bench/check_local_by_default2.out",
http_server: true,
exit_code: 1,
});
#[test]
fn recursive_permissions_pledge() {
let output = util::deno_cmd()

View File

@ -456,3 +456,16 @@ itest!(bundle_ignore_directives {
args: "bundle subdir/mod1.ts",
output: "bundle_ignore_directives.test.out",
});
itest!(check_local_by_default {
args: "bundle --quiet bundle/check_local_by_default.ts",
output: "bundle/check_local_by_default.out",
http_server: true,
});
itest!(check_local_by_default2 {
args: "bundle --quiet bundle/check_local_by_default2.ts",
output: "bundle/check_local_by_default2.out",
http_server: true,
exit_code: 1,
});

View File

@ -78,3 +78,16 @@ fn relative_home_dir() {
assert!(output.status.success());
assert_eq!(output.stdout, b"");
}
itest!(check_local_by_default {
args: "cache --quiet cache/check_local_by_default.ts",
output: "cache/check_local_by_default.out",
http_server: true,
});
itest!(check_local_by_default2 {
args: "cache --quiet cache/check_local_by_default2.ts",
output: "cache/check_local_by_default2.out",
http_server: true,
exit_code: 1,
});

View File

@ -479,3 +479,52 @@ fn skip_rebundle() {
assert!(output.status.success());
assert_eq!(output.stdout, "Hello World\n".as_bytes());
}
#[test]
fn check_local_by_default() {
let _guard = util::http_server();
let dir = TempDir::new();
let exe = if cfg!(windows) {
dir.path().join("welcome.exe")
} else {
dir.path().join("welcome")
};
let status = util::deno_cmd()
.current_dir(util::root_path())
.arg("compile")
.arg("--unstable")
.arg("--output")
.arg(&exe)
.arg(util::testdata_path().join("./compile/check_local_by_default.ts"))
.status()
.unwrap();
assert!(status.success());
}
#[test]
fn check_local_by_default2() {
let _guard = util::http_server();
let dir = TempDir::new();
let exe = if cfg!(windows) {
dir.path().join("welcome.exe")
} else {
dir.path().join("welcome")
};
let output = util::deno_cmd()
.current_dir(util::root_path())
.env("NO_COLOR", "1")
.arg("compile")
.arg("--unstable")
.arg("--output")
.arg(&exe)
.arg(util::testdata_path().join("./compile/check_local_by_default2.ts"))
.output()
.unwrap();
assert!(!output.status.success());
let stdout = String::from_utf8(output.stdout).unwrap();
let stderr = String::from_utf8(output.stderr).unwrap();
assert!(stdout.is_empty());
assert!(stderr.contains(
r#"error: TS2322 [ERROR]: Type '12' is not assignable to type '"b"'."#
));
}

View File

@ -66,3 +66,16 @@ itest!(v8_flags_eval {
args: "eval --v8-flags=--expose-gc console.log(typeof(gc))",
output: "v8_flags.js.out",
});
itest!(check_local_by_default {
args: "eval --quiet import('http://localhost:4545/subdir/type_error.ts').then(console.log);",
output: "eval/check_local_by_default.out",
http_server: true,
});
itest!(check_local_by_default2 {
args: "eval --quiet import('./eval/check_local_by_default2.ts').then(console.log);",
output: "eval/check_local_by_default2.out",
http_server: true,
exit_code: 1,
});

View File

@ -41,9 +41,11 @@ fn install_basic() {
assert_eq!(content.chars().last().unwrap(), '\n');
if cfg!(windows) {
assert!(content.contains(r#""run" "http://localhost:4545/echo.ts""#));
assert!(
content.contains(r#""run" "--check" "http://localhost:4545/echo.ts""#)
);
} else {
assert!(content.contains(r#"run 'http://localhost:4545/echo.ts'"#));
assert!(content.contains(r#"run --check 'http://localhost:4545/echo.ts'"#));
}
}
@ -79,9 +81,11 @@ fn install_custom_dir_env_var() {
let content = fs::read_to_string(file_path).unwrap();
if cfg!(windows) {
assert!(content.contains(r#""run" "http://localhost:4545/echo.ts""#));
assert!(
content.contains(r#""run" "--check" "http://localhost:4545/echo.ts""#)
);
} else {
assert!(content.contains(r#"run 'http://localhost:4545/echo.ts'"#));
assert!(content.contains(r#"run --check 'http://localhost:4545/echo.ts'"#));
}
}
@ -157,3 +161,50 @@ fn installer_test_remote_module_run() {
.trim()
.ends_with("hello, foo"));
}
#[test]
fn check_local_by_default() {
let _guard = util::http_server();
let temp_dir = TempDir::new();
let temp_dir_str = temp_dir.path().to_string_lossy().to_string();
let status = util::deno_cmd()
.current_dir(temp_dir.path())
.arg("install")
.arg(util::testdata_path().join("./install/check_local_by_default.ts"))
.envs([
("HOME", temp_dir_str.as_str()),
("USERPROFILE", temp_dir_str.as_str()),
("DENO_INSTALL_ROOT", ""),
])
.status()
.unwrap();
assert!(status.success());
}
#[test]
fn check_local_by_default2() {
let _guard = util::http_server();
let temp_dir = TempDir::new();
let temp_dir_str = temp_dir.path().to_string_lossy().to_string();
let output = util::deno_cmd()
.current_dir(temp_dir.path())
.arg("install")
.arg(util::testdata_path().join("./install/check_local_by_default2.ts"))
.envs([
("HOME", temp_dir_str.as_str()),
("NO_COLOR", "1"),
("USERPROFILE", temp_dir_str.as_str()),
("DENO_INSTALL_ROOT", ""),
])
.output()
.unwrap();
assert!(!output.status.success());
let stdout = String::from_utf8(output.stdout).unwrap();
let stderr = String::from_utf8(output.stderr).unwrap();
assert!(stdout.is_empty());
assert!(stderr.contains(
r#"error: TS2322 [ERROR]: Type '12' is not assignable to type '"b"'."#
));
}

View File

@ -963,7 +963,7 @@ itest!(no_check_decorators {
});
itest!(check_remote {
args: "run --quiet --reload no_check_remote.ts",
args: "run --quiet --reload --check=all no_check_remote.ts",
output: "no_check_remote.ts.disabled.out",
exit_code: 1,
http_server: true,

View File

@ -374,3 +374,16 @@ itest!(uncaught_errors {
output: "test/uncaught_errors.out",
exit_code: 1,
});
itest!(check_local_by_default {
args: "test --quiet test/check_local_by_default.ts",
output: "test/check_local_by_default.out",
http_server: true,
});
itest!(check_local_by_default2 {
args: "test --quiet test/check_local_by_default2.ts",
output: "test/check_local_by_default2.out",
http_server: true,
exit_code: 1,
});

View File

@ -0,0 +1,5 @@
[WILDCARD]
[WILDCARD]/bench/check_local_by_default.ts
benchmark time (avg) (min … max) p75 p99 p995
------------------------------------------------- -----------------------------

View File

@ -0,0 +1,3 @@
import * as a from "http://localhost:4545/subdir/type_error.ts";
console.log(a.a);

View File

@ -0,0 +1,4 @@
error: TS2322 [ERROR]: Type '12' is not assignable to type '"b"'.
const b: "b" = 12;
^
at [WILDCARD]bench/check_local_by_default2.ts:3:7

View File

@ -0,0 +1,6 @@
import * as a from "http://localhost:4545/subdir/type_error.ts";
const b: "b" = 12;
console.log(a.a);
console.log(b);

View File

@ -0,0 +1,6 @@
// deno-fmt-ignore-file
// deno-lint-ignore-file
// This code was bundled using `deno bundle` and it's not recommended to edit it manually
console.log(12);

View File

@ -0,0 +1,3 @@
import * as a from "http://localhost:4545/subdir/type_error.ts";
console.log(a.a);

View File

@ -0,0 +1,4 @@
error: TS2322 [ERROR]: Type '12' is not assignable to type '"b"'.
const b: "b" = 12;
^
at [WILDCARD]bundle/check_local_by_default2.ts:3:7

View File

@ -0,0 +1,6 @@
import * as a from "http://localhost:4545/subdir/type_error.ts";
const b: "b" = 12;
console.log(a.a);
console.log(b);

View File

View File

@ -0,0 +1,3 @@
import * as a from "http://localhost:4545/subdir/type_error.ts";
console.log(a.a);

View File

@ -0,0 +1,4 @@
error: TS2322 [ERROR]: Type '12' is not assignable to type '"b"'.
const b: "b" = 12;
^
at [WILDCARD]cache/check_local_by_default2.ts:3:7

View File

@ -0,0 +1,6 @@
import * as a from "http://localhost:4545/subdir/type_error.ts";
const b: "b" = 12;
console.log(a.a);
console.log(b);

View File

@ -0,0 +1,3 @@
import * as a from "http://localhost:4545/subdir/type_error.ts";
console.log(a.a);

View File

@ -0,0 +1,6 @@
import * as a from "http://localhost:4545/subdir/type_error.ts";
const b: "b" = 12;
console.log(a.a);
console.log(b);

View File

@ -0,0 +1 @@
Module { a: 12 }

View File

@ -0,0 +1,4 @@
error: TS2322 [ERROR]: Type '12' is not assignable to type '"b"'.
const b: "b" = 12;
^
at [WILDCARD]eval/check_local_by_default2.ts:3:7

View File

@ -0,0 +1,6 @@
import * as a from "http://localhost:4545/subdir/type_error.ts";
const b: "b" = 12;
console.log(a.a);
console.log(b);

View File

@ -0,0 +1,3 @@
import * as a from "http://localhost:4545/subdir/type_error.ts";
console.log(a.a);

View File

@ -0,0 +1,6 @@
import * as a from "http://localhost:4545/subdir/type_error.ts";
const b: "b" = 12;
console.log(a.a);
console.log(b);

View File

@ -0,0 +1,4 @@
running 0 tests from ./test/check_local_by_default.ts
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out ([WILDCARD])

View File

@ -0,0 +1,3 @@
import * as a from "http://localhost:4545/subdir/type_error.ts";
console.log(a.a);

View File

@ -0,0 +1,4 @@
error: TS2322 [ERROR]: Type '12' is not assignable to type '"b"'.
const b: "b" = 12;
^
at [WILDCARD]test/check_local_by_default2.ts:3:7

View File

@ -0,0 +1,6 @@
import * as a from "http://localhost:4545/subdir/type_error.ts";
const b: "b" = 12;
console.log(a.a);
console.log(b);

View File

@ -308,11 +308,9 @@ fn resolve_shim_data(
// we should avoid a default branch here to ensure we continue to cover any
// changes to this flag.
match flags.type_check_mode {
TypeCheckMode::All => (),
TypeCheckMode::None => executable_args.push("--no-check".to_string()),
TypeCheckMode::Local => {
executable_args.push("--no-check=remote".to_string())
}
TypeCheckMode::All => executable_args.push("--check=all".to_string()),
TypeCheckMode::None => {}
TypeCheckMode::Local => executable_args.push("--check".to_string()),
}
if flags.unstable {
@ -515,11 +513,12 @@ mod tests {
println!("this is the file path {:?}", content);
if cfg!(windows) {
assert!(content.contains(
r#""run" "--unstable" "http://localhost:4545/echo_server.ts""#
r#""run" "--check" "--unstable" "http://localhost:4545/echo_server.ts""#
));
} else {
assert!(content
.contains(r#"run --unstable 'http://localhost:4545/echo_server.ts'"#));
assert!(content.contains(
r#"run --check --unstable 'http://localhost:4545/echo_server.ts'"#
));
}
}
@ -540,7 +539,7 @@ mod tests {
assert_eq!(shim_data.name, "echo_server");
assert_eq!(
shim_data.args,
vec!["run", "http://localhost:4545/echo_server.ts",]
vec!["run", "--check", "http://localhost:4545/echo_server.ts",]
);
}
@ -561,7 +560,7 @@ mod tests {
assert_eq!(shim_data.name, "subdir");
assert_eq!(
shim_data.args,
vec!["run", "http://localhost:4545/subdir/main.ts",]
vec!["run", "--check", "http://localhost:4545/subdir/main.ts",]
);
}
@ -582,7 +581,7 @@ mod tests {
assert_eq!(shim_data.name, "echo_test");
assert_eq!(
shim_data.args,
vec!["run", "http://localhost:4545/echo_server.ts",]
vec!["run", "--check", "http://localhost:4545/echo_server.ts",]
);
}
@ -615,7 +614,6 @@ mod tests {
"--allow-read",
"--allow-net",
"--quiet",
"--no-check",
"--compat",
"http://localhost:4545/echo_server.ts",
"--foobar",
@ -642,7 +640,12 @@ mod tests {
assert_eq!(
shim_data.args,
vec!["run", "--no-prompt", "http://localhost:4545/echo_server.ts",]
vec![
"run",
"--check",
"--no-prompt",
"http://localhost:4545/echo_server.ts",
]
);
}
@ -665,7 +668,12 @@ mod tests {
assert_eq!(
shim_data.args,
vec!["run", "--allow-all", "http://localhost:4545/echo_server.ts",]
vec![
"run",
"--allow-all",
"--check",
"http://localhost:4545/echo_server.ts",
]
);
}
@ -828,9 +836,8 @@ mod tests {
if cfg!(windows) {
// TODO: see comment above this test
} else {
assert!(
content.contains(r#"run 'http://localhost:4545/echo_server.ts' '"'"#)
);
assert!(content
.contains(r#"run --check 'http://localhost:4545/echo_server.ts' '"'"#));
}
}
@ -949,9 +956,10 @@ mod tests {
}
assert!(file_path.exists());
let mut expected_string = format!("run '{}'", &file_module_string);
let mut expected_string = format!("run --check '{}'", &file_module_string);
if cfg!(windows) {
expected_string = format!("\"run\" \"{}\"", &file_module_string);
expected_string =
format!("\"run\" \"--check\" \"{}\"", &file_module_string);
}
let content = fs::read_to_string(file_path).unwrap();

View File

@ -275,9 +275,10 @@ pub fn compile_to_runtime_flags(
lock_write: false,
lock: None,
log_level: flags.log_level,
type_check_mode: TypeCheckMode::All,
future_type_check_mode: FutureTypeCheckMode::None,
has_no_check_flag: false,
has_check_flag: false,
type_check_mode: TypeCheckMode::Local,
future_type_check_mode: FutureTypeCheckMode::None,
compat: flags.compat,
unsafely_ignore_certificate_errors: flags
.unsafely_ignore_certificate_errors