plumbing strict overrides flag

Summary: This diff plumbs the new flag through to suppress or show the strict overrides error in typechecking.

Reviewed By: kinto0

Differential Revision: D60085379

fbshipit-source-id: e8a706541fb154d2948fe69172ad59c79a12ea32

source/analysis/analysisError.ml

@@ -4063,13 +4063,18 @@ let filter ~resolution errors =
   List.filter ~f:(fun error -> not (should_filter error)) errors
 
 
-let suppress ~mode ~ignore_codes error =
+let suppress
+    ~mode
+    ~ignore_codes
+    ~type_check_controls:{ EnvironmentControls.TypeCheckControls.include_strict_override_errors; _ }
+    error
+  =
   let suppress_in_strict ({ kind; _ } as error) =
     if due_to_analysis_limitations error then
       true
     else
       match kind with
-      | InvalidOverride { decorator = MissingOverride; _ } -> true
+      | InvalidOverride { decorator = MissingOverride; _ } -> not include_strict_override_errors
       | IncompleteType _ ->
           (* TODO(T42467236): Ungate this when ready to codemod upgrade *)
           true

source/analysis/analysisError.mli

@@ -601,7 +601,12 @@ val deduplicate : t list -> t list
 
 val filter : resolution:GlobalResolution.t -> t list -> t list
 
-val suppress : mode:Source.mode -> ignore_codes:int list -> t -> bool
+val suppress
+  :  mode:Source.mode ->
+  ignore_codes:int list ->
+  type_check_controls:EnvironmentControls.TypeCheckControls.t ->
+  t ->
+  bool
 
 val dequalify : Reference.t Reference.Map.t -> resolution:GlobalResolution.t -> t -> t
 

source/analysis/postprocessing.ml

@@ -117,10 +117,11 @@ let filter_errors
     ~mode
     ~global_resolution
     ~typecheck_flags:{ Source.TypecheckFlags.ignore_codes; _ }
+    ~type_check_controls
     errors_by_define
   =
   let filter errors =
-    let keep_error error = not (Error.suppress ~mode ~ignore_codes error) in
+    let keep_error error = not (Error.suppress ~mode ~ignore_codes ~type_check_controls error) in
     List.filter ~f:keep_error errors
   in
   List.map errors_by_define ~f:(fun errors -> filter errors |> List.sort ~compare:Error.compare)
@@ -151,10 +152,11 @@ let run_on_source
   in
   let global_resolution = TypeEnvironment.ReadOnly.global_resolution environment in
   let controls = TypeEnvironment.ReadOnly.controls environment in
+  let type_check_controls = EnvironmentControls.type_check_controls controls in
   let mode = Source.mode ~configuration:(EnvironmentControls.configuration controls) ~local_mode in
-  filter_errors ~mode ~global_resolution ~typecheck_flags errors_by_define
+  filter_errors ~mode ~global_resolution ~typecheck_flags ~type_check_controls errors_by_define
   |> add_local_mode_errors ~define:(Source.top_level_define_node source) source
-  |> suppress_errors_based_on_comments ~controls:(EnvironmentControls.type_check_controls controls)
+  |> suppress_errors_based_on_comments ~controls:type_check_controls
   |> List.map
        ~f:(Error.dequalify (Preprocessing.dequalify_map source) ~resolution:global_resolution)
   |> List.sort ~compare:Error.compare

source/analysis/test/analysisErrorTest.ml

@@ -698,11 +698,27 @@ let test_filter context =
 
 
 let test_suppress _ =
+  let type_check_controls =
+    {
+      EnvironmentControls.TypeCheckControls.include_type_errors = true;
+      include_local_annotations = true;
+      include_readonly_errors = true;
+      include_strict_override_errors = true;
+      include_unawaited_awaitable_errors = true;
+      debug = true;
+      include_suppressed_errors = true;
+      no_validation_on_class_lookup_failure = true;
+    }
+  in
   let assert_suppressed mode ?(ignore_codes = []) ?(signature = mock_signature) ?location kind =
-    assert_equal true (Error.suppress ~mode ~ignore_codes (error ~signature ?location kind))
+    assert_equal
+      true
+      (Error.suppress ~mode ~ignore_codes ~type_check_controls (error ~signature ?location kind))
   in
   let assert_not_suppressed mode ?(ignore_codes = []) ?(signature = mock_signature) kind =
-    assert_equal false (Error.suppress ~mode ~ignore_codes (error ~signature kind))
+    assert_equal
+      false
+      (Error.suppress ~mode ~ignore_codes ~type_check_controls (error ~signature kind))
   in
   (* Test different modes. *)
   assert_not_suppressed Source.Debug (missing_return Type.Top);

source/analysis/test/integration/overrideTest.ml

@@ -8,6 +8,69 @@
 open OUnit2
 open IntegrationTest
 
+let test_suppression =
+  test_list
+    [
+      (* suppressed in unsafe mode *)
+      labeled_test_case __FUNCTION__ __LINE__
+      @@ assert_default_type_errors
+           {|
+      class C:
+        def f(self) -> None:
+          pass
+
+      class D(C):
+        def f(self) -> None:
+          pass
+    |}
+           [];
+      (* suppressed in strict mode until we codemod *)
+      labeled_test_case __FUNCTION__ __LINE__
+      @@ assert_strict_type_errors
+           {|
+      class C:
+        def f(self) -> None:
+          pass
+
+      class D(C):
+        def f(self) -> None:
+          pass
+    |}
+           [];
+      (* show error in strict mode if flag is set *)
+      labeled_test_case __FUNCTION__ __LINE__
+      @@ assert_strict_type_errors
+           ~enable_strict_override_check:true
+           {|
+      class C:
+        def f(self) -> None:
+          pass
+
+      class D(C):
+        def f(self) -> None:
+          pass
+    |}
+           [
+             "Invalid override [40]: `test.D.f` is not decorated with @override, but overrides a \
+              method with the same name in superclasses of `D`.";
+           ];
+      (* do not show error in unsafe mode if flag is set *)
+      labeled_test_case __FUNCTION__ __LINE__
+      @@ assert_default_type_errors
+           ~enable_strict_override_check:true
+           {|
+      class C:
+        def f(self) -> None:
+          pass
+
+      class D(C):
+        def f(self) -> None:
+          pass
+    |}
+           [];
+    ]
+
+
 let test_extra_overriding_parameter =
   test_list
     [
@@ -316,4 +379,4 @@ let test_extra_overriding_parameter =
     ]
 
 
-let () = "override" >::: [test_extra_overriding_parameter] |> Test.run
+let () = "override" >::: [test_suppression; test_extra_overriding_parameter] |> Test.run