summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--perl_checker.src/parser_helper.ml33
-rw-r--r--perl_checker.src/test/suggest_better.t3
-rw-r--r--perl_checker.src/test/various_errors.t2
3 files changed, 30 insertions, 8 deletions
diff --git a/perl_checker.src/parser_helper.ml b/perl_checker.src/parser_helper.ml
index c317f98..b4afdaf 100644
--- a/perl_checker.src/parser_helper.ml
+++ b/perl_checker.src/parser_helper.ml
@@ -840,6 +840,22 @@ msgstr \"\"
) sorted_pot_strings ;
close_out fd
+let fake_string_from_String_l l = String.concat "$foo" (List.map fst l)
+let fake_string_option_from_expr = function
+ | String(l, _) -> Some(fake_string_from_String_l l)
+ | Raw_string(s, _) -> Some s
+ | _ -> None
+
+let check_system_call = function
+ | "mkdir" :: l ->
+ let has_p = List.exists (str_begins_with "-p") l in
+ let has_m = List.exists (str_begins_with "-m") l in
+ if has_p && has_m then ()
+ else if has_p then warn_rule "you can replace system(\"mkdir -p ...\") with mkdir_p(...)"
+ else if has_m then warn_rule "you can replace system(\"mkdir -m <mode> ...\") with mkdir(..., <mode>)"
+ else warn_rule "you can replace system(\"mkdir ...\") with mkdir(...)"
+ | _ -> ()
+
let call_raw force_non_builtin_func (e, para) =
let check_anonymous_block f = function
| [ Anonymous_sub _ ; Deref (I_hash, _) ] ->
@@ -946,12 +962,17 @@ let call_raw force_non_builtin_func (e, para) =
| "system" ->
(match un_parenthesize_full_l para with
- | [ String(l, _) ] ->
- if List.exists (fun (s, _) -> String.contains s '\'' || String.contains s '"') l &&
- not (List.exists (fun (s, _) -> List.exists (String.contains s) [ '<' ; '>' ; '&' ; ';']) l) then
- warn_rule "instead of quoting parameters you should give a list of arguments"
- | _ -> ())
-
+ | [ e ] ->
+ (match fake_string_option_from_expr e with
+ | Some s ->
+ if List.exists (String.contains s) [ '\'' ; char_quote ] &&
+ not (List.exists (String.contains s) [ '<' ; '>' ; '&' ; ';']) then
+ warn_rule "instead of quoting parameters you should give a list of arguments";
+ check_system_call (split_at ' ' s)
+ | None -> ())
+ | l ->
+ let l' = filter_some_with fake_string_option_from_expr l in
+ check_system_call l')
| _ -> ()
);
diff --git a/perl_checker.src/test/suggest_better.t b/perl_checker.src/test/suggest_better.t
index 4b8d2d5..e98e64f 100644
--- a/perl_checker.src/test/suggest_better.t
+++ b/perl_checker.src/test/suggest_better.t
@@ -100,3 +100,6 @@ $xxx ? $yyy : () you may use if_() here
beware that the short-circuit semantic of ?: is not kept
if you want to keep the short-circuit behaviour, replace () with @{[]} and there will be no warning anymore
+system(qq(foo "$xxx")) instead of quoting parameters you should give a list of arguments
+
+system("mkdir", $xxx) you can replace system("mkdir ...") with mkdir(...)
diff --git a/perl_checker.src/test/various_errors.t b/perl_checker.src/test/various_errors.t
index f9c4089..dfac926 100644
--- a/perl_checker.src/test/various_errors.t
+++ b/perl_checker.src/test/various_errors.t
@@ -39,8 +39,6 @@ pop @l, 1 pop is expecting an array and nothing e
pop $xxx pop is expecting an array and nothing else
-system(qq(foo "$xxx")) instead of quoting parameters you should give a list of arguments
-
my (@l2, $xxx) = @l; @l2 takes all the arguments, $xxx is undef in any case
$bad undeclared variable $bad