Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix [declarations] not suppressing errors correctly & rename to [silence] #8105

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/services/inference/init_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ let load_lib_files ~sig_cx ~options ~reader files =
(fun (exclude_syms, results) file ->
let lib_file = File_key.LibFile file in
let lint_severities = options.Options.opt_lint_severities in
let file_options = Options.file_options options in
let%lwt result = parse_lib_file ~reader options file in
Lwt.return
(match result with
Expand Down Expand Up @@ -95,7 +94,6 @@ let load_lib_files ~sig_cx ~options ~reader files =
ast
~exclude_syms
~lint_severities
~file_options:(Some file_options)
~file_sig
in
let errors = Context.errors cx in
Expand Down
4 changes: 0 additions & 4 deletions src/services/inference/merge_service.ml
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ let merge_context_generic ~options ~reader ~get_ast_unsafe ~get_file_sig_unsafe
let (master_cx, dep_cxs, file_reqs) = reqs_of_component ~reader component required in
let metadata = Context.metadata_of_options options in
let lint_severities = Options.lint_severities options in
let file_options = Some (Options.file_options options) in
let strict_mode = Options.strict_mode options in
let get_aloc_table_unsafe =
Parsing_heaps.Reader_dispatcher.get_sig_ast_aloc_table_unsafe ~reader
Expand All @@ -134,7 +133,6 @@ let merge_context_generic ~options ~reader ~get_ast_unsafe ~get_file_sig_unsafe
Merge_js.merge_component
~metadata
~lint_severities
~file_options
~strict_mode
~file_sigs
~phase
Expand Down Expand Up @@ -221,7 +219,6 @@ let merge_contents_context ~reader options file ast info file_sig =
in
let metadata = Context.metadata_of_options options in
let lint_severities = Options.lint_severities options in
let file_options = Some (Options.file_options options) in
let strict_mode = Options.strict_mode options in
let get_aloc_table_unsafe =
Parsing_heaps.Reader_dispatcher.get_sig_ast_aloc_table_unsafe ~reader
Expand All @@ -230,7 +227,6 @@ let merge_contents_context ~reader options file ast info file_sig =
Merge_js.merge_component
~metadata
~lint_severities
~file_options
~strict_mode
~file_sigs
~get_ast_unsafe:(fun _ -> (comments, aloc_ast))
Expand Down
9 changes: 7 additions & 2 deletions src/typing/errors/error_suppressions.ml
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@ let in_node_modules ~root ~file_options loc =
| None -> false
| Some (file, options) -> Files.is_within_node_modules ~root ~options (File_key.to_string file)

let in_declarations ~file_options loc =
match Option.both (Loc.source loc) file_options with
| None -> false
| Some (file, options) -> Files.is_declaration options (File_key.to_string file)

let check ~root ~file_options (err : Loc.t Errors.printable_error) (suppressions : t) (unused : t)
=
let locs =
Expand All @@ -176,11 +181,11 @@ let check ~root ~file_options (err : Loc.t Errors.printable_error) (suppressions
* without a source. *)
|> List.filter (fun loc -> Option.is_some (Loc.source loc))
in
(* Ignore lint errors from node modules. *)
(* Ignore lint errors from node modules, and all errors from declarations directories. *)
let ignore =
match Errors.kind_of_printable_error err with
| Errors.LintError _ -> in_node_modules ~root ~file_options (Errors.loc_of_printable_error err)
| _ -> false
| _ -> in_declarations ~file_options (Errors.loc_of_printable_error err)
in
if ignore then
None
Expand Down
2 changes: 0 additions & 2 deletions src/typing/merge_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,6 @@ let detect_non_voidable_properties cx =
let merge_component
~metadata
~lint_severities
~file_options
~strict_mode
~file_sigs
~get_ast_unsafe
Expand Down Expand Up @@ -416,7 +415,6 @@ let merge_component
comments
ast
~lint_severities
~file_options
~file_sig
in
(cx :: cxs, tast :: tasts, FilenameMap.add filename cx impl_cxs))
Expand Down
1 change: 0 additions & 1 deletion src/typing/merge_js.mli
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ end
val merge_component :
metadata:Context.metadata ->
lint_severities:Severity.severity LintSettings.t ->
file_options:Files.options option ->
strict_mode:StrictModeSettings.t ->
file_sigs:File_sig.With_ALoc.t Utils_js.FilenameMap.t ->
get_ast_unsafe:(File_key.t -> get_ast_return) ->
Expand Down
28 changes: 7 additions & 21 deletions src/typing/type_inference_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -333,23 +333,9 @@ let scan_for_lint_suppressions =
Context.add_severity_cover cx (Context.file cx) severity_cover;
Context.add_lint_suppressions cx suppression_locs

let scan_for_suppressions cx lint_severities file_options comments =
let filename = File_key.to_string (Context.file cx) in
let declaration =
match file_options with
| Some file_options -> Files.is_declaration file_options filename
| None -> false
in
if declaration then
(* Declaration mode.
* We don't report any warnings or errors. *)
Context.remove_all_errors cx
else
(* Scan comments for line suppressions. *)
scan_for_error_suppressions cx comments;
scan_for_lint_suppressions cx lint_severities comments;

()
let scan_for_suppressions cx lint_severities comments =
scan_for_error_suppressions cx comments;
scan_for_lint_suppressions cx lint_severities comments

let add_require_tvars =
let add cx desc loc =
Expand Down Expand Up @@ -385,7 +371,7 @@ let add_require_tvars =

(* build module graph *)
(* Lint suppressions are handled iff lint_severities is Some. *)
let infer_ast ~lint_severities ~file_options ~file_sig cx filename comments aloc_ast =
let infer_ast ~lint_severities ~file_sig cx filename comments aloc_ast =
assert (Context.is_checked cx);

Flow_js.Cache.clear ();
Expand Down Expand Up @@ -432,7 +418,7 @@ let infer_ast ~lint_severities ~file_options ~file_sig cx filename comments aloc
(* infer *)
Flow_js.flow_t cx (init_exports, local_exports_var);
let typed_statements = infer_core cx aloc_statements in
scan_for_suppressions cx lint_severities file_options comments;
scan_for_suppressions cx lint_severities comments;

let module_t = Import_export.mk_module_t cx reason in
Context.add_module cx module_ref module_t;
Expand Down Expand Up @@ -471,7 +457,7 @@ let with_libdef_builtins cx f =
a) symbols from prior library loads are suppressed if found,
b) bindings are added as properties to the builtin object
*)
let infer_lib_file ~exclude_syms ~lint_severities ~file_options ~file_sig cx ast =
let infer_lib_file ~exclude_syms ~lint_severities ~file_sig cx ast =
let aloc_ast = Ast_loc_utils.loc_to_aloc_mapper#program ast in
let (_, _, comments) = ast in
let (_, aloc_statements, _) = aloc_ast in
Expand All @@ -487,7 +473,7 @@ let infer_lib_file ~exclude_syms ~lint_severities ~file_options ~file_sig cx ast

with_libdef_builtins cx (fun () ->
ignore (infer_core cx aloc_statements : (ALoc.t, ALoc.t * Type.t) Ast.Statement.t list);
scan_for_suppressions cx lint_severities file_options comments);
scan_for_suppressions cx lint_severities comments);

( module_scope
|> Scope.(
Expand Down
2 changes: 0 additions & 2 deletions src/typing/type_inference_js.mli
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
(* Lint suppressions are handled iff lint_severities is Some. *)
val infer_ast :
lint_severities:Severity.severity LintSettings.t ->
file_options:Files.options option ->
file_sig:File_sig.With_ALoc.t ->
Context.t ->
File_key.t ->
Expand All @@ -20,7 +19,6 @@ val infer_ast :
val infer_lib_file :
exclude_syms:SSet.t ->
lint_severities:Severity.severity LintSettings.t ->
file_options:Files.options option ->
file_sig:File_sig.With_ALoc.t ->
Context.t ->
(Loc.t, Loc.t) Flow_ast.program ->
Expand Down
10 changes: 9 additions & 1 deletion tests/config_declarations/config_declarations.exp
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,13 @@ Unused suppression comment.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


Warning ------------------------------------------------------------------------------------------------------- B.js:1:1

Found 1 error and 1 warning
Unused suppression comment.

1| // $FlowFixMe
^^^^^^^^^^^^^



Found 1 error and 2 warnings
14 changes: 14 additions & 0 deletions tests/config_declarations_react/.flowconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[ignore]

[include]

[libs]

[declarations]
<PROJECT_ROOT>/node_modules/react-native/.*

[options]
suppress_comment=.*\\$FlowFixMe
suppress_comment=.*\\$FlowIssue
suppress_comment=\\(.\\|\n\\)*\\$FlowNewLine
include_warnings=true
14 changes: 14 additions & 0 deletions tests/config_declarations_react/A.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Due to [declarations], this won't error
const str2str = require('react-native/str2str')

// But this identical one will
const str2str_loud = require('react-d3/str2str')

// Calling str2str with a string is correct
const foo: string = str2str(' bar ');

// But not with a number
const fooNum: number = str2str(39); // Would not error if using [untyped]!

// trim2 should still error as well
str2str(39);
5 changes: 5 additions & 0 deletions tests/config_declarations_react/B.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const React = require('React');

// Will error. Ensuring this still errors properly despite the react-native decl
const AsyncMode: boolean = React.unstable_AsyncMode;

63 changes: 63 additions & 0 deletions tests/config_declarations_react/config_declarations_react.exp
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
Error ------------------------------------------------------------------------------------------------------- A.js:11:24

Cannot assign `str2str(...)` to `fooNum` because string [1] is incompatible with number [2].

A.js:11:24
11| const fooNum: number = str2str(39); // Would not error if using [untyped]!
^^^^^^^^^^^

References:
node_modules/react-native/str2str.js:7:40
7| module.exports = function str2str(foo: string) {
^^^^^^ [1]
A.js:11:15
11| const fooNum: number = str2str(39); // Would not error if using [untyped]!
^^^^^^ [2]


Error ------------------------------------------------------------------------------------------------------- A.js:11:32

Cannot call `str2str` with `39` bound to `foo` because number [1] is incompatible with string [2].

A.js:11:32
11| const fooNum: number = str2str(39); // Would not error if using [untyped]!
^^ [1]

References:
node_modules/react-native/str2str.js:7:40
7| module.exports = function str2str(foo: string) {
^^^^^^ [2]


Error -------------------------------------------------------------------------------------------------------- A.js:14:9

Cannot call `str2str` with `39` bound to `foo` because number [1] is incompatible with string [2].

A.js:14:9
14| str2str(39);
^^ [1]

References:
node_modules/react-native/str2str.js:7:40
7| module.exports = function str2str(foo: string) {
^^^^^^ [2]


Error -------------------------------------------------------------------------------------------------------- B.js:4:28

Cannot get `React.unstable_AsyncMode` because property `unstable_AsyncMode` is missing in exports [1].

4| const AsyncMode: boolean = React.unstable_AsyncMode;
^^^^^^^^^^^^^^^^^^^^^^^^


Error ---------------------------------------------------------------------------- node_modules/react-d3/str2str.js:4:19

Cannot get `React.unstable_AsyncMode` because property `unstable_AsyncMode` is missing in exports [1].

4| const AsyncMode = React.unstable_AsyncMode;
^^^^^^^^^^^^^^^^^^^^^^^^



Found 5 errors

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

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

Empty file.
11 changes: 11 additions & 0 deletions tests/config_declarations_react/node_modules/react/package.json

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