Adding SourceExp transition to AL

Reviewed By: martintrojer

Differential Revision: D15181376

fbshipit-source-id: f235a1375

infer/src/clang/cTL.ml

@@ -28,12 +28,20 @@ type transitions =
   | Cond
   | PointerToDecl  (** stmt to decl *)
   | Protocol  (** decl to decl *)
+  | SourceExpr
 [@@deriving compare]
 
 let is_transition_to_successor trans =
   match trans with
-  | Body | InitExpr | FieldName _ | Fields | ParameterName _ | ParameterPos _ | Parameters | Cond
+  | Body
-    ->
+  | InitExpr
+  | FieldName _
+  | Fields
+  | ParameterName _
+  | ParameterPos _
+  | Parameters
+  | Cond
+  | SourceExpr ->
       true
   | Super | PointerToDecl | Protocol | AccessorForProperty _ ->
       false
@@ -150,6 +158,8 @@ module Debug = struct
           Format.pp_print_string fmt "Protocol"
       | PointerToDecl ->
           Format.pp_print_string fmt "PointerToDecl"
+      | SourceExpr ->
+          Format.pp_print_string fmt "SourceExpr"
     in
     match trans_opt with Some trans -> pp_aux fmt trans | None -> Format.pp_print_char fmt '_'
 
@@ -780,6 +790,15 @@ let transition_stmt_to_decl_via_pointer stmt =
       []
 
 
+let transition_stmt_to_stmt_via_source_expr stmt =
+  let open Clang_ast_t in
+  match stmt with
+  | OpaqueValueExpr (_, _, _, ovei) -> (
+    match ovei.ovei_source_expr with Some st -> [Stmt st] | None -> [] )
+  | _ ->
+      []
+
+
 let transition_via_parameters an =
   let open Clang_ast_t in
   match an with
@@ -926,6 +945,8 @@ let next_state_via_transition an trans =
       transition_stmt_to_stmt_via_condition st
   | Stmt st, PointerToDecl ->
       transition_stmt_to_decl_via_pointer st
+  | Stmt st, SourceExpr ->
+      transition_stmt_to_stmt_via_source_expr st
   | an, ParameterName name ->
       transition_via_parameter_name an name
   | an, ParameterPos pos ->

infer/src/clang/cTL.mli

@@ -28,6 +28,7 @@ type transitions =
   | Cond
   | PointerToDecl  (** stmt to decl *)
   | Protocol  (** decl to decl *)
+  | SourceExpr
 [@@deriving compare]
 
 (* In formulas below prefix

infer/src/clang/ctl_lexer.mll

@@ -83,6 +83,7 @@ rule token = parse
   | "InitExpr" {INIT_EXPR}
   | "Cond" {COND}
   | "PointerToDecl" {POINTER_TO_DECL}
+   | "SourceExpr" {SOURCE_EXPR}
   | id { IDENTIFIER (Lexing.lexeme lexbuf) }
   | '"' { read_string (Buffer.create 80) lexbuf }
   | _ { raise (SyntaxError ("Unexpected char: '" ^ (Lexing.lexeme lexbuf) ^"'")) }

infer/src/clang/ctl_parser.mly

@@ -86,6 +86,7 @@
 %token PARAMETER_NAME
 %token PARAMETER_POS
 %token POINTER_TO_DECL
+%token SOURCE_EXPR
 %token PROTOCOL
 %token EOF
 
@@ -249,6 +250,7 @@ transition_label:
   | PARAMETER_NAME alexp { Some (CTL.ParameterName $2) }
   | PARAMETER_POS alexp { Some (CTL.ParameterPos $2) }
   | POINTER_TO_DECL { Some CTL.PointerToDecl }
+  | SOURCE_EXPR { Some CTL.SourceExpr}
   | PROTOCOL { Some CTL.Protocol }
   ;
 

infer/tests/codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m

@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2019-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+#import <Foundation/Foundation.h>
+
+@interface UselessClass : NSObject
+- (int)a;
+- (int)b;
+@end
+
+@implementation UselessClass
+- (int)a {
+  return 4;
+}
+- (int)b {
+  return 5;
+}
+@end
+
+static int add(int x, int y) { return x + y; }
+
+@interface ClassWithPrivateMethods : NSObject
+- (void)publicMethodThatDoesntUseIvars;
+@end
+
+@implementation ClassWithPrivateMethods {
+  int _someNum;
+  UselessClass* _someClass;
+}
+
+- (void)publicMethodThatDoesntUseIvars {
+  NSLog(@"Hello World!");
+}
+
+- (void)_okayMethod1 {
+  // This is fine because we are referencing an ivar.
+  _someNum = 4;
+}
+
+- (void)_okayMethod2 {
+  self->_someNum = 5;
+}
+
+- (int)_okayMethod3 {
+  return add(_someClass.a, _someClass.b);
+}
+
+- (void)_badMethod1 {
+  // This isn't referencing an ivar...there is no reason for this.
+  NSLog(@"Hi Mom!");
+}
+
+- (void)_badMethod2 {
+  ClassWithPrivateMethods* const c = [ClassWithPrivateMethods new];
+  // This is unnecessary because we aren't referencing self.
+  c->_someNum = 5;
+}
+
+@end

infer/tests/codetoanalyze/objc/linters-for-test-only/al_definitions/linters_example.al

@@ -817,3 +817,43 @@ DEFINE-CHECKER CONST_NAMING = {
 
   SET message = "That's a const";
 };
+
+DEFINE-CHECKER UNNECESSARY_OBJC_INSTANCE_METHOD = {
+
+  LET is_in_implementation =
+    is_in_objc_implementation_named(REGEXP(".*")) OR
+    is_in_objc_category_implementation_named(REGEXP(".*"));
+
+
+ LET dereference_self =
+      declaration_ref_name(REGEXP("self")) HOLDS-EVENTUALLY;
+
+LET  dereference_self_in_opaque_value_expr =
+          IN-NODE OpaqueValueExpr WITH-TRANSITION SourceExpr
+           (dereference_self)
+          HOLDS-EVENTUALLY;
+
+LET dereference_self_in_method_decl =
+           IN-NODE ObjCMethodDecl WITH-TRANSITION Body
+           (dereference_self  OR dereference_self_in_opaque_value_expr)
+           HOLDS-EVENTUALLY;
+
+  LET is_private_method =
+    declaration_has_name(REGEXP("^_")) AND
+    NOT is_objc_method_exposed AND
+    (NOT dereference_self_in_method_decl)
+    AND is_in_implementation;
+
+
+
+  SET report_when =
+    WHEN
+    is_private_method
+    HOLDS-IN-NODE ObjCMethodDecl;
+
+
+  SET name = "Unnecessary Objective-C Method";
+  SET message = "This is an unnecessary Objective-C Method";
+  SET severity = "ERROR";
+  SET mode = "ON";
+};

infer/tests/codetoanalyze/objc/linters-for-test-only/issues.exp

@@ -204,6 +204,18 @@ codetoanalyze/objc/linters-for-test-only/InContextOfMethodsTest.m, objc_block_1,
 codetoanalyze/objc/linters-for-test-only/InContextOfMethodsTest.m, objc_block_2, 19, TEST_IN_BLOCK_CONTEXT, no_bucket, WARNING, []
 codetoanalyze/objc/linters-for-test-only/InSubclassExample.m, HappySadView2::makeBadAction, 33, IN_SUBCLASS_TEST, no_bucket, WARNING, []
 codetoanalyze/objc/linters-for-test-only/PrivateAPIChecker.m, TestView::methoddd, 19, TEST_SELECTOR, no_bucket, WARNING, []
+codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m, ClassWithPrivateMethods::_badMethod1, 51, UNNECESSARY_OBJC_INSTANCE_METHOD, no_bucket, ERROR, []
+codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m, ClassWithPrivateMethods::_badMethod2, 56, UNNECESSARY_OBJC_INSTANCE_METHOD, no_bucket, ERROR, []
+codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m, ClassWithPrivateMethods::_badMethod2, 57, CONST_NAMING, no_bucket, WARNING, []
+codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m, ClassWithPrivateMethods::_badMethod2, 57, TEST_ALL_METHODS, no_bucket, WARNING, []
+codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m, ClassWithPrivateMethods::_okayMethod3, 47, TEST_RETURN_METHOD, no_bucket, WARNING, []
+codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m, ClassWithPrivateMethods::publicMethodThatDoesntUseIvars, 34, TEST_IS_METHOD_EXPOSED, no_bucket, WARNING, []
+codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m, UselessClass::a, 10, TEST_RETURN_METHOD, no_bucket, WARNING, []
+codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m, UselessClass::a, 15, TEST_IS_METHOD_EXPOSED, no_bucket, WARNING, []
+codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m, UselessClass::a, 15, TEST_RETURN_METHOD, no_bucket, WARNING, []
+codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m, UselessClass::b, 11, TEST_RETURN_METHOD, no_bucket, WARNING, []
+codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m, UselessClass::b, 18, TEST_IS_METHOD_EXPOSED, no_bucket, WARNING, []
+codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m, UselessClass::b, 18, TEST_RETURN_METHOD, no_bucket, WARNING, []
 codetoanalyze/objc/linters-for-test-only/const.m, Linters_dummy_method, 7, CONST_NAMING, no_bucket, WARNING, []
 codetoanalyze/objc/linters-for-test-only/const.m, Linters_dummy_method, 7, TEST_VAR_TYPE_CHECK, no_bucket, WARNING, []
 codetoanalyze/objc/linters-for-test-only/const.m, Linters_dummy_method, 8, CONST_NAMING, no_bucket, WARNING, []