- warn <always true> || ... (eg: 1 || foo())

- warn <always false> || ...
- warn <always true> && ...
- warn <always false> && ...
- suggest @$foo instead of @{$foo}
- suggest $foo->[0] instead of ${$foo}[0]

3 files changed, 40 insertions, 7 deletions

diff --git a/perl_checker.src/parser.mly b/perl_checker.src/parser.mly
index 4d73fca..5b78351 100644
--- a/perl_checker.src/parser.mly
+++ b/perl_checker.src/parser.mly
@@ -349,8 +349,8 @@ diamond:
 
 subscripted: /* Some kind of subscripted expression */
 | variable PKG_SCOPE bracket_subscript {sp_0($2); sp_0($3); Call(Too_complex, [fst $3]), sp_pos_range $1 $3} /* $foo::{something} */
-| scalar bracket_subscript             {sp_0($2); to_Deref_with(I_hash , I_scalar, from_scalar $1, fst                $2), sp_pos_range $1 $2} /* $foo{bar} */
-| scalar arrayref                      {sp_0($2); to_Deref_with(I_array, I_scalar, from_scalar $1, only_one_array_ref $2), sp_pos_range $1 $2} /* $array[$element] */
+| scalar bracket_subscript             {sp_0($2); check_scalar_subscripted $1; to_Deref_with(I_hash , I_scalar, from_scalar $1, fst                $2), sp_pos_range $1 $2} /* $foo{bar} */
+| scalar arrayref                      {sp_0($2); check_scalar_subscripted $1; to_Deref_with(I_array, I_scalar, from_scalar $1, only_one_array_ref $2), sp_pos_range $1 $2} /* $array[$element] */
 | term ARROW bracket_subscript         {sp_0($2); sp_0($3); check_arrow_needed $1 $2; to_Deref_with(I_hash , I_scalar, sndfst $1, fst                $3), sp_pos_range $1 $3} /* somehref->{bar} */
 | term ARROW arrayref                  {sp_0($2); sp_0($3); check_arrow_needed $1 $2; to_Deref_with(I_array, I_scalar, sndfst $1, only_one_array_ref $3), sp_pos_range $1 $3} /* somearef->[$element] */
 | term ARROW parenthesized             {sp_0($2); sp_0($3); to_Deref_with(I_func , I_scalar, sndfst $1, List(sndfst $3)), sp_pos_range $1 $3} /* $subref->(@args) */
@@ -360,8 +360,8 @@ subscripted: /* Some kind of subscripted expression */
 
 restricted_subscripted: /* Some kind of subscripted expression */
 | variable PKG_SCOPE bracket_subscript {sp_0($2); sp_0($3); Call(Too_complex, [fst $3]), sp_pos_range $1 $3} /* $foo::{something} */
-| scalar bracket_subscript             {sp_0($2); to_Deref_with(I_hash , I_scalar, from_scalar $1, fst                $2), sp_pos_range $1 $2} /* $foo{bar} */
-| scalar arrayref                      {sp_0($2); to_Deref_with(I_array, I_scalar, from_scalar $1, only_one_array_ref $2), sp_pos_range $1 $2} /* $array[$element] */
+| scalar bracket_subscript             {sp_0($2); check_scalar_subscripted $1; to_Deref_with(I_hash , I_scalar, from_scalar $1, fst                $2), sp_pos_range $1 $2} /* $foo{bar} */
+| scalar arrayref                      {sp_0($2); check_scalar_subscripted $1; to_Deref_with(I_array, I_scalar, from_scalar $1, only_one_array_ref $2), sp_pos_range $1 $2} /* $array[$element] */
 | restricted_subscripted bracket_subscript        {sp_0($2); to_Deref_with(I_hash , I_scalar, fst $1, fst                $2), sp_pos_range $1 $2} /* $foo->[bar]{baz} */
 | restricted_subscripted arrayref                 {sp_0($2); to_Deref_with(I_array, I_scalar, fst $1, only_one_array_ref $2), sp_pos_range $1 $2} /* $foo->[$bar][$baz] */
 | restricted_subscripted parenthesized            {sp_0($2); to_Deref_with(I_func , I_scalar, fst $1, List(sndfst $2)), sp_pos_range $1 $2} /* $foo->{bar}(@args) */
diff --git a/perl_checker.src/parser_helper.ml b/perl_checker.src/parser_helper.ml
index 832ad76..f16ca14 100644
--- a/perl_checker.src/parser_helper.ml
+++ b/perl_checker.src/parser_helper.ml
@@ -58,6 +58,19 @@ let rec un_parenthesize_full = function
   | List[e] -> un_parenthesize_full e
   | e -> e
 
+let is_always_true = function
+  | Num(n, _) -> float_of_string n <> 0.
+  | Raw_string(s, _) -> s <> ""
+  | String(l, _) -> l <> []
+  | Ref _ -> true
+  | _ -> false
+
+let is_always_false = function
+  | Num(n, _) -> float_of_string n = 0.
+  | Raw_string(s, _) -> s = ""
+  | String(l, _) -> l = []
+  | _ -> false
+
 let not_complex e =
   if is_parenthesized e then true else
   let rec not_complex_ op = function
@@ -296,7 +309,7 @@ let check_parenthesized_first_argexpr_with_Ident ident ((prio, e), _ as ex) =
       (match e with
       | [e] when is_parenthesized e -> ()
       | _ -> warn_rule "use parentheses around argument (otherwise it might cause syntax errors if the package is \"require\"d and not \"use\"d")
-  | Ident(None, word, _) when List.mem word ["ref"] ->
+  | Ident(None, word, _) when List.mem word ["ref" ; "readlink"] ->
       if prio <> P_tok then warn_rule "use parentheses around argument"
   | _ -> ());
   check_parenthesized_first_argexpr (string_of_Ident ident) ex
@@ -318,6 +331,11 @@ let check_arrow_needed ((_, e), _) ter =
   | Deref_with _ -> warn (sndsnd ter) "the arrow \"->\" is unneeded"
   | _ -> ()
 
+let check_scalar_subscripted (e, _) =
+  match e with
+  | Deref(I_scalar, Deref _) -> warn_rule "for complex dereferencing, use \"->\""
+  | _ -> ()
+
 let check_ternary_paras(cond, a, b) =
   let rec dont_need_short_circuit_rec = function
     | Num _
@@ -432,6 +450,9 @@ let deref_raw context e =
   | Raw_string(s, pos) -> 
       let fq, ident = split_name_or_fq_name s in
       Ident(fq, ident, pos)
+  | Deref(I_scalar, (Ident _ as ident)) ->
+      warn_rule (sprintf "%s{$%s} can be written %s$%s" (context2s context) (string_of_Ident ident) (context2s context) (string_of_Ident ident));
+      e
   | _ -> e
   in Deref(context, e)
 
@@ -508,6 +529,11 @@ let cook_call_op(op, para, pos) =
   | "=", [ Deref(I_star, (Ident _ as f1)); (Anonymous_sub _ as sub) ] ->
       sub_declaration (f1, "") [ sub ]
 
+  | "||", e :: _ when is_always_true  e -> warn_rule "<constant> || ... is the same as <constant>"; call
+  | "&&", e :: _ when is_always_false e -> warn_rule "<constant> && ... is the same as <constant>"; call
+  | "||", e :: _ when is_always_false e -> warn_rule "<constant> || ... is the same as ..."; call
+  | "&&", e :: _ when is_always_true  e -> warn_rule "<constant> && ... is the same as ..."; call
+
   | _ -> 
       call
 
diff --git a/perl_checker.src/parser_helper.mli b/perl_checker.src/parser_helper.mli
index cbb04e6..adb7a05 100644
--- a/perl_checker.src/parser_helper.mli
+++ b/perl_checker.src/parser_helper.mli
@@ -10,15 +10,20 @@ val var_STDOUT : Types.fromparser
 val split_name_or_fq_name : string -> string option * string
 val is_var_dollar_ : Types.fromparser -> bool
 val is_var_number_match : Types.fromparser -> bool
+val non_scalar_context : Types.context -> bool
+val is_scalar_context : Types.context -> bool
+val is_not_a_scalar : Types.fromparser -> bool
+val is_a_scalar : Types.fromparser -> bool
 val is_parenthesized : Types.fromparser -> bool
 val un_parenthesize : Types.fromparser -> Types.fromparser
 val un_parenthesize_full : Types.fromparser -> Types.fromparser
+val is_always_true : Types.fromparser -> bool
+val is_always_false : Types.fromparser -> bool
 val not_complex : Types.fromparser -> bool
 val not_simple : Types.fromparser -> bool
 val string_of_Ident : Types.fromparser -> string
 val context2s : Types.context -> string
 val variable2s : Types.context * string -> string
-val non_scalar_context : Types.context -> bool
 val is_same_fromparser : Types.fromparser -> Types.fromparser -> bool
 val from_scalar : Types.fromparser * 'a -> Types.fromparser
 val from_array : Types.fromparser * 'a -> Types.fromparser
@@ -66,6 +71,7 @@ val check_hash_subscript :
   ('a * Types.fromparser) * ('b * (int * int)) -> unit
 val check_arrow_needed :
   ('a * Types.fromparser) * 'b -> 'c * ('d * (int * int)) -> unit
+val check_scalar_subscripted : Types.fromparser * 'a -> unit
 val check_ternary_paras :
   Types.fromparser * Types.fromparser * Types.fromparser ->
   Types.fromparser list
@@ -94,7 +100,6 @@ val only_one_array_ref :
 val only_one_in_List :
   ('a * Types.fromparser) * ('b * (int * int)) -> Types.fromparser
 val is_only_one_in_List : Types.fromparser list -> bool
-val is_not_a_scalar : Types.fromparser -> bool
 val maybe_to_Raw_string : Types.fromparser -> Types.fromparser
 val to_List : Types.fromparser list -> Types.fromparser
 val deref_arraylen : Types.fromparser -> Types.fromparser
@@ -134,6 +139,8 @@ val to_Call_op_ :
   'b * (int * int) -> ('a * Types.fromparser) * ('b * (int * int))
 val followed_by_comma :
   ('a * Types.fromparser list) * 'b -> bool * 'c -> Types.fromparser list
+val pot_strings : (string, string list) Hashtbl.t
+val pot_strings_and_file : (string, string) Hashtbl.t
 val po_comments : string list ref
 val po_comment : string * 'a -> unit
 val check_format_a_la_printf : string -> int -> unit