Skip to content

Commit b10078b

Browse files
Wilfredfacebook-github-bot
authored andcommitted
Don't fire the duplicate return lint on positive exit codes
Reviewed By: periodic1236 Differential Revision: D38948922 fbshipit-source-id: 9e667fcc33c9df7a5608b61b87686a790667f16c
1 parent 7f18108 commit b10078b

File tree

3 files changed

+66
-17
lines changed

3 files changed

+66
-17
lines changed

hphp/hack/src/lints/linter_branches_return_same_value.ml

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,22 @@
88
open Aast
99
open Hh_prelude
1010

11+
(**
12+
Lint on functions/methods that always return the same value. This
13+
is usually a copy-paste error.
14+
15+
function some_prediate(Whatever $value): bool {
16+
if ($value->foo()) {
17+
return false;
18+
}
19+
20+
if ($value->bar()) {
21+
return false;
22+
}
23+
24+
return false; // oops!
25+
} *)
26+
1127
let is_expr_same ((_, _, expr1) : Tast.expr) ((_, _, expr2) : Tast.expr) : bool
1228
=
1329
match (expr1, expr2) with
@@ -26,6 +42,21 @@ let is_expr_same ((_, _, expr1) : Tast.expr) ((_, _, expr2) : Tast.expr) : bool
2642
true
2743
| _ -> false
2844

45+
(* Does this value look like a success exit code? We don't want to
46+
lint on code like this:
47+
48+
function my_main(): int {
49+
if (quick_check()) { return 0; }
50+
51+
normal_work();
52+
return 0;
53+
} *)
54+
let is_success_ish ((_, _, e_) : Tast.expr) : bool =
55+
match e_ with
56+
| Aast.Int "0" -> true
57+
| Aast.Class_const (_, (_, "SUCCESS")) -> true
58+
| _ -> false
59+
2960
let rec are_return_expressions_same (ret_list : Tast.expr list) : bool =
3061
match ret_list with
3162
| expr1 :: expr2 :: tail ->
@@ -61,25 +92,20 @@ let get_return_expr_visitor =
6192
let get_return_exprs (stmts : Tast.stmt list) : Tast.expr list =
6293
get_return_expr_visitor#on_block () stmts
6394

95+
let check_block (block : Tast.block) : unit =
96+
let ret_list = get_return_exprs block in
97+
match ret_list with
98+
| expr1 :: _ :: _ when are_return_expressions_same ret_list ->
99+
if not (is_success_ish expr1) then
100+
List.iter ret_list ~f:(fun (_, pos, _) ->
101+
Lints_errors.branches_return_same_value pos)
102+
| _ -> ()
103+
64104
let handler =
65105
object
66106
inherit Tast_visitor.handler_base
67107

68-
method! at_fun_def _env f =
69-
let ret_list = get_return_exprs f.fd_fun.f_body.fb_ast in
70-
if List.length ret_list > 1 then (
71-
if are_return_expressions_same ret_list then
72-
List.iter ret_list ~f:(fun (_, pos, _) ->
73-
Lints_errors.branches_return_same_value pos)
74-
) else
75-
()
76-
77-
method! at_method_ _env m =
78-
let ret_list = get_return_exprs m.m_body.fb_ast in
79-
if List.length ret_list > 1 then (
80-
if are_return_expressions_same ret_list then
81-
List.iter ret_list ~f:(fun (_, pos, _) ->
82-
Lints_errors.branches_return_same_value pos)
83-
) else
84-
()
108+
method! at_fun_def _env f = check_block f.fd_fun.f_body.fb_ast
109+
110+
method! at_method_ _env m = check_block m.m_body.fb_ast
85111
end
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?hh
2+
3+
enum MyExitCode: int {
4+
SUCCESS = 0;
5+
FAILURE = 1;
6+
}
7+
8+
function main_function_early_terminate(): int {
9+
if (1 === 1) {
10+
return 0;
11+
}
12+
13+
return 0;
14+
}
15+
16+
function main_function_early_terminate_enum(): int {
17+
if (1 === 1) {
18+
return MyExitCode::SUCCESS;
19+
}
20+
21+
return MyExitCode::SUCCESS;
22+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
No lint errors

0 commit comments

Comments
 (0)