Requiring methods in interfaces to forgo bodies

Summary: Prior to this diff, the full fidelity parser handled method declarations in interfaces incorrectly: it a) threw error2015 on interface methods without bodies, and b) allowed interface methods to have bodies. In this diff I reversed both of these behaviors. The updated full fidelity parser behavior is demonstrated in test_interface_method_errors.(php/exp).

Reviewed By: ericlippert

Differential Revision: D5326581

fbshipit-source-id: 0174fda5cf6967069042d41424da97e4b9eecb3f

hphp/hack/src/parser/full_fidelity_parser_errors.ml

@@ -140,6 +140,10 @@ let token_kind node =
   | Token t -> Some (PositionedToken.kind t)
   | _ -> None
 
+(* Helper function for common code pattern *)
+let is_token_kind node kind =
+  (token_kind node) = Some kind
+
 let rec containing_classish_kind parents =
   match parents with
   | [] -> None
@@ -258,6 +262,18 @@ let methodish_has_body node =
     not (is_missing body)
   | _ -> false
 
+(* By checking the third parent of a methodish node, tests whether the methodish
+ * node is inside an interface. *)
+let methodish_inside_interface parents =
+  match parents with
+  | _ :: _ :: p3 :: _ ->
+    begin match syntax p3 with
+    | ClassishDeclaration { classish_keyword; _ } ->
+      is_token_kind classish_keyword TokenKind.Interface
+    | _ -> false
+    end
+  | _ -> false
+
 (* Test whether node is an abstract method with a body.
  * Here node is the methodish node *)
 let methodish_abstract_with_body node =
@@ -266,12 +282,18 @@ let methodish_abstract_with_body node =
   is_abstract && has_body
 
 (* Test whether node is a non-abstract method without a body.
- * Here node is the methodish node *)
-let methodish_non_abstract_without_body node =
-  let non_abstract = not (methodish_contains_abstract node) in
+ * Here node is the methodish node
+ * And methods inside interfaces are inherently considered abstract *)
+let methodish_non_abstract_without_body node parents =
+  let non_abstract = not (methodish_contains_abstract node
+      || methodish_inside_interface parents) in
   let not_has_body = not (methodish_has_body node) in
   non_abstract && not_has_body
 
+let methodish_in_interface_has_body node parents =
+  methodish_inside_interface parents &&
+      not (is_missing node.methodish_function_body)
+
 (* Test whether node is a method that is both abstract and private
  *)
 let methodish_abstract_conflict_with_private node =
@@ -400,10 +422,6 @@ let first_function_name parents =
     | _ -> None
   end
 
-(* Helper function for common code pattern *)
-  let is_token_kind node kind =
-    (token_kind node) = Some kind
-
 (* Test if (a list_expression is the left child of a binary_expression,
  * and the operator is '=') *)
 let is_left_of_simple_assignment le_node p1 =
@@ -466,14 +484,17 @@ let methodish_errors node parents =
       SyntaxError.error2014 fun_body in
     let fun_semicolon = md.methodish_semicolon in
     let errors =
-      produce_error errors methodish_non_abstract_without_body node
+      produce_error errors (methodish_non_abstract_without_body node) parents
       SyntaxError.error2015 fun_semicolon in
     let errors =
       produce_error errors methodish_abstract_conflict_with_private
       node SyntaxError.error2016 modifiers in
     let errors =
       produce_error errors methodish_abstract_conflict_with_final
       node SyntaxError.error2019 modifiers in
+    let errors =
+      produce_error errors (methodish_in_interface_has_body md) parents
+      SyntaxError.error2041 md.methodish_function_body in
     errors
   | _ -> [ ]
 

hphp/hack/src/parser/full_fidelity_syntax_error.ml

@@ -168,3 +168,4 @@ let error2039 classish_keyword classish_name function_name = Printf.sprintf
 let error2040 = "Invalid use of 'list(...)'. A list expression may only be " ^
   "used as the left side of a simple assignment, the value clause of a " ^
   "foreach loop, or a list item nested inside another list expression."
+let error2041 = "A method inside an interface may not have a body."

hphp/hack/test/full_fidelity/cases/test_interface_method_errors.exp

@@ -0,0 +1,3 @@
+(5,22)-(5,24) A method inside an interface may not have a body.
+(9,22)-(9,22) A non-abstract method must have a body.
+(14,23)-(14,23) A non-abstract method must have a body.

hphp/hack/test/full_fidelity/cases/test_interface_method_errors.php

@@ -0,0 +1,16 @@
+<?hh // strict
+
+interface I {
+  function i1(): int; // legal
+  function i2(): int { } // error2041
+}
+
+class C {
+  function c1(): void; // error2015
+  function c2(): void { } // legal
+}
+
+trait T {
+  function t1() : void; // error2015
+  function t2() : void { } // legal
+}

hphp/hack/test/full_fidelity/full_fidelity_unit_test.ml

@@ -182,6 +182,7 @@ let error_tests =
     "test_object_creation_errors";
     "test_classish_inside_function_errors";
     "test_list_expression_errors";
+    "test_interface_method_errors";
   ] ~f:mapper
 
 let test_data = minimal_tests @ error_tests @