diff --git a/infer/src/clang/cTL.ml b/infer/src/clang/cTL.ml index 49073415c..35d7c578d 100644 --- a/infer/src/clang/cTL.ml +++ b/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 -> diff --git a/infer/src/clang/cTL.mli b/infer/src/clang/cTL.mli index 4b873d9ac..92c13e28e 100644 --- a/infer/src/clang/cTL.mli +++ b/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 diff --git a/infer/src/clang/ctl_lexer.mll b/infer/src/clang/ctl_lexer.mll index d54d59144..ca8613165 100644 --- a/infer/src/clang/ctl_lexer.mll +++ b/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) ^"'")) } diff --git a/infer/src/clang/ctl_parser.mly b/infer/src/clang/ctl_parser.mly index 07bf26da2..f856ff1b6 100644 --- a/infer/src/clang/ctl_parser.mly +++ b/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 } ; diff --git a/infer/tests/codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m b/infer/tests/codetoanalyze/objc/linters-for-test-only/PrivateInstanceMethod.m new file mode 100644 index 000000000..58267cd6f --- /dev/null +++ b/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 + +@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 diff --git a/infer/tests/codetoanalyze/objc/linters-for-test-only/al_definitions/linters_example.al b/infer/tests/codetoanalyze/objc/linters-for-test-only/al_definitions/linters_example.al index 1f9cb770c..f8b579c1c 100644 --- a/infer/tests/codetoanalyze/objc/linters-for-test-only/al_definitions/linters_example.al +++ b/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"; +}; diff --git a/infer/tests/codetoanalyze/objc/linters-for-test-only/issues.exp b/infer/tests/codetoanalyze/objc/linters-for-test-only/issues.exp index 1d19ec71b..69764d880 100644 --- a/infer/tests/codetoanalyze/objc/linters-for-test-only/issues.exp +++ b/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, []