diff options
| -rw-r--r-- | perl_checker.src/parser_helper.ml | 33 | ||||
| -rw-r--r-- | perl_checker.src/test/suggest_better.t | 3 | ||||
| -rw-r--r-- | perl_checker.src/test/various_errors.t | 2 |
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 |