From f37d7371879e2b1e2d923ec12762430b3d1937fc Mon Sep 17 00:00:00 2001 From: Pascal Rigaux Date: Wed, 25 Apr 2007 15:16:22 +0000 Subject: re-sync after the big svn loss --- src/Makefile | 4 +- src/config_file.ml | 4 +- src/global_checks.ml | 6 +-- src/lexer.mll | 5 ++- src/parser.mly | 6 +-- src/parser_helper.ml | 97 +++++++++++++++++++++++++++++++-------- src/parser_helper.mli | 10 ++++- src/perl_checker.html.pl | 112 +++++++++++++++++++++++++++++++++++----------- src/test/return_value.t | 8 ++++ src/test/suggest_better.t | 2 + src/test/various_errors.t | 4 ++ src/tree.ml | 19 ++++---- src/types.mli | 2 + 13 files changed, 215 insertions(+), 64 deletions(-) (limited to 'src') diff --git a/src/Makefile b/src/Makefile index 22a45a6..5df6a31 100644 --- a/src/Makefile +++ b/src/Makefile @@ -6,7 +6,7 @@ RESULT = perl_checker BCSUFFIX = _debug SOURCES = types.mli build.ml common.ml flags.ml config_file.ml info.ml parser_helper.ml parser.mly lexer.mll tree.ml global_checks.ml perl_checker.ml LIBS = unix -VENDORLIB = $(shell dirname `pwd`) +fake_packages_dir = $(shell dirname `pwd`)/fake_packages DEBUG = 1 default: TAGS build_ml build.ml debug-code native-code perl_checker.html @@ -17,7 +17,7 @@ build_ml: build.ml: date '+let date = "%s"' > $@ - echo 'let fake_packages_dir = "'$(VENDORLIB)'/perl_checker_fake_packages"' >> $@ + echo 'let fake_packages_dir = "'$(fake_packages_dir)'"' >> $@ echo 'let debugging = $(DEBUG) > 0' >> $@ %.html: %.html.pl diff --git a/src/config_file.ml b/src/config_file.ml index a5ee94f..efb6fb3 100644 --- a/src/config_file.ml +++ b/src/config_file.ml @@ -19,8 +19,8 @@ let read dir = let config = fold_lines (fun config line -> match words line with - | [ "Basedir"; ".." ] -> { config with basedir = Some 1 } - | [ "Basedir"; "../.." ] -> { config with basedir = Some 2 } + | [ "Basedir"; ".." ] -> { basedir = Some 1 } + | [ "Basedir"; "../.." ] -> { basedir = Some 2 } | [] -> config (* blank line *) | [ "Ignore"; pkg ] | [ pkg ] (* the deprecated form *) diff --git a/src/global_checks.ml b/src/global_checks.ml index a63e652..4a97221 100644 --- a/src/global_checks.ml +++ b/src/global_checks.ml @@ -174,7 +174,7 @@ let is_global_var context ident = | _ -> false) | I_hash -> (match ident with - | "ENV" | "SIG" -> true + | "ENV" | "SIG" | "INC" -> true | _ -> false) | I_star -> (match ident with @@ -183,7 +183,7 @@ let is_global_var context ident = | _ -> false) | I_func -> (match ident with - | "-b" | "-c" | "-d" | "-e" | "-f" | "-l" | "-r" | "-s" | "-w" | "-x" + | "-b" | "-c" | "-d" | "-e" | "-f" | "-l" | "-r" | "-s" | "-w" | "-x" | "-z" | "-t" | "abs" | "alarm" | "atan2" | "bless" | "caller" | "chdir" | "chmod" | "chomp" | "chop" | "chown" | "chr" | "chroot" | "close" | "closedir" | "cos" | "crypt" | "defined" | "delete" | "die" @@ -399,7 +399,7 @@ let check_variables vars t = | Call_op("if infix", [List [My_our _]; List [Num("0", _)]], _) -> None (* special allowed case *) | Call_op(op, List (My_our _ :: _) :: _, pos) - | Call_op(op, My_our _ :: _, pos) + | Call_op(op, My_our("my", _, _) :: _, pos) | Call_op(op, Call_op("local", _, _) :: _, pos) -> if op <> "=" then warn_with_pos [Warn_traps] pos (sprintf "applying %s on a new initialized variable is wrong" op); None diff --git a/src/lexer.mll b/src/lexer.mll index f416499..dfa8561 100644 --- a/src/lexer.mll +++ b/src/lexer.mll @@ -78,6 +78,7 @@ let rec concat_bareword_paren accu = function | PO_COMMENT _ :: _ (* the check will be done on this PO_COMMENT *) | BAREWORD("N", _) :: PAREN(_) :: _ + | BAREWORD("P", _) :: PAREN(_) :: _ | BAREWORD("N_", _) :: PAREN(_) :: _ -> concat_bareword_paren (e :: accu) l | _ -> @@ -800,7 +801,7 @@ and string = parse next string lexbuf } | "'" { string_escape_useful := Left true ; next string lexbuf } -| [^ '\n' '\\' '"' '$' '@']+ { next string lexbuf } +| [^ '\n' '\\' '"' ''' '$' '@']+ { next string lexbuf } | eof { die_in_string lexbuf "Unterminated_string" } and delimited_string = parse @@ -917,7 +918,7 @@ and string_escape = parse | 'Q' { warn [Warn_complex_expressions] lexbuf ("don't use \\Q, use quotemeta instead"); string_escape_useful := Left true; next_s ("\\" ^ lexeme lexbuf) (Stack.pop next_rule) lexbuf } -| ['b' 'f' 'a' 'r'] { string_escape_useful := Left true; next_s ("\\" ^ lexeme lexbuf) (Stack.pop next_rule) lexbuf } +| ['a' 'c' 'b' 'f' 'l' 'r'] { string_escape_useful := Left true; next_s ("\\" ^ lexeme lexbuf) (Stack.pop next_rule) lexbuf } | ['$' '@' '%' '{' '[' ':'] { if !string_escape_useful = Left false then string_escape_useful := Right (lexeme_char lexbuf 0) ; next_s (lexeme lexbuf) (Stack.pop next_rule) lexbuf diff --git a/src/parser.mly b/src/parser.mly index a9bf396..78fb461 100644 --- a/src/parser.mly +++ b/src/parser.mly @@ -104,7 +104,7 @@ prog: lines EOF {fst $1.any} lines: /* A collection of "lines" in the program */ | { default_esp ([], true) } | sideff { new_1esp ([$1.any], false) $1 } -| line lines { if fst $2.any <> [] then mcontext_check_none "value is dropped" $1.any $1; new_esp $2.mcontext ($1.any @ fst $2.any, snd $2.any) $1 $2 } +| line lines { if fst $2.any <> [] then mcontext_check_none_no_drop_always $1 $2; new_esp $2.mcontext ($1.any @ fst $2.any, snd $2.any) $1 $2 } line: | decl { new_1esp [$1.any] $1 } @@ -190,8 +190,8 @@ listexpr: /* Basic list expressions */ | argexpr %prec PREC_LOW {$1} expr: /* Ordinary expressions; logical combinations */ -| expr AND expr {sp_p($2); sp_p($3); mcontext_check M_bool $1; mcontext_check_none "value should be dropped" [$3.any.expr] $3; to_Call_op_ M_none P_and "and" [ prio_lo P_and $1; prio_lo_after P_and $3 ] $1 $3} -| expr OR expr {sp_p($2); sp_p($3); mcontext_check M_bool $1; mcontext_check_none "value should be dropped" [$3.any.expr] $3; to_Call_op_ M_none P_or "or" [ prio_lo P_or $1; prio_lo_after P_or $3 ] $1 $3} +| expr AND expr {sp_p($2); sp_p($3); mcontext_check M_bool $1; mcontext_check_none_should_drop [$3.any.expr] $3; to_Call_op_ M_none P_and "and" [ prio_lo P_and $1; prio_lo_after P_and $3 ] $1 $3} +| expr OR expr {sp_p($2); sp_p($3); mcontext_check M_bool $1; mcontext_check_none_should_drop [$3.any.expr] $3; to_Call_op_ M_none P_or "or" [ prio_lo P_or $1; prio_lo_after P_or $3 ] $1 $3} | argexpr %prec PREC_LOW { new_1pesp $1.any.priority (List $1.any.expr) $1 } argexpr: /* Expressions are a list of terms joined by commas */ diff --git a/src/parser_helper.ml b/src/parser_helper.ml index 43d60a4..d798e14 100644 --- a/src/parser_helper.ml +++ b/src/parser_helper.ml @@ -56,7 +56,9 @@ let is_a_scalar = function | Num _ | Raw_string _ | String _ - | Call(Deref(I_func, Ident(None, "N", _)), _) -> true + | Call(Deref(I_func, Ident(None, "N", _)), _) + | Call(Deref(I_func, Ident(None, "P", _)), _) + -> true | My_our(_, [ context, _ ], _) | Deref_with(_, context, _, _) | Deref(context, _) -> is_scalar_context context @@ -415,7 +417,7 @@ let function_to_context word_alone = function | "any" | "every" -> M_bool | "find_index" -> M_int | "each_index" -> M_none - | "N" | "N_" -> M_string + | "N" | "P" | "N_" -> M_string | "chop" | "chomp" | "push" | "unshift" -> M_none | "hex" | "length" | "time" | "fork" | "getppid" -> M_int @@ -425,7 +427,7 @@ let function_to_context word_alone = function | "split" -> M_array | "shift" | "pop" -> M_unknown_scalar - | "die" | "return" | "redo" | "next" | "last" -> M_unknown + | "die" | "return" | "redo" | "next" | "last" | "exit" -> M_break_control_flow | "caller" -> M_mixed [M_string ; M_list] | "ref" -> M_ref M_unknown_scalar @@ -535,6 +537,7 @@ let check_ternary_paras(cond, a, b) = | String ([(_, List [])], _) -> true | Call(Deref(I_func, Ident(None, "N", _)), [ List(String _ :: l) ]) + | Call(Deref(I_func, Ident(None, "P", _)), [ List(String _ :: l) ]) | Call_op(".", l, _) | Ref(I_hash, List l) | List l -> List.for_all dont_need_short_circuit_rec l @@ -695,6 +698,7 @@ let to_Local esp = Some(context, ident) | Deref(I_scalar, Ident _) | Deref(I_array, Ident _) + | Deref(I_hash, Ident _) | Deref(I_star, Ident _) | Deref_with(I_hash, I_scalar, Ident _, _) | Deref_with(I_hash, I_scalar, Deref(I_scalar, _), _) @@ -832,6 +836,7 @@ let followed_by_comma expr true_comma = | l, Ident(None, s, pos) -> l @ [Raw_string(s, pos)] | _ -> expr +type msgid_t = No_plural of string | With_plural of string * string let pot_strings = Hashtbl.create 16 let po_comments = ref [] @@ -872,6 +877,7 @@ msgstr \"\" \"MIME-Version: 1.0\\n\" \"Content-Type: text/plain; charset=CHARSET\\n\" \"Content-Transfer-Encoding: 8-bit\\n\" +\"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\\n\" ") ; @@ -883,12 +889,12 @@ msgstr \"\" | c -> output_char fd c in let sorted_pot_strings = List.sort (fun (_, pos_a) (_, pos_b) -> compare pos_a pos_b) - (Hashtbl.fold (fun k (v, _) l -> (k,v) :: l) pot_strings [] ) in - List.iter (fun (s, _) -> - match Hashtbl.find_all pot_strings s with + (Hashtbl.fold (fun msgid (pos, _) l -> (msgid,pos) :: l) pot_strings [] ) in + List.iter (fun (msgid, _) -> + match Hashtbl.find_all pot_strings msgid with | [] -> () | l -> - List.iter (fun _ -> Hashtbl.remove pot_strings s) l ; + List.iter (fun _ -> Hashtbl.remove pot_strings msgid) l ; List.iter (fun po_comment -> output_string fd ("#. " ^ po_comment ^ "\n")) (collect snd l); @@ -896,10 +902,19 @@ msgstr \"\" fprintf fd "#: %s\n" (String.concat " " (List.map Info.pos2s_for_po pos_l)) ; output_string fd "#, c-format\n" ; - output_string fd (if String.contains s '\n' then "msgid \"\"\n\"" else "msgid \"") ; - String.iter print_formatted_char s ; - output_string fd "\"\n" ; - output_string fd "msgstr \"\"\n\n" + let output_it msgid_s s = + output_string fd (msgid_s ^ if String.contains s '\n' then " \"\"\n\"" else " \"") ; + String.iter print_formatted_char s ; + output_string fd "\"\n" + in + match msgid with + | No_plural s_ -> + output_it "msgid" s_ ; + output_string fd "msgstr \"\"\n\n" + | With_plural (s1, sn) -> + output_it "msgid" s1 ; + output_it "msgid_plural" sn ; + output_string fd "msgstr[0] \"\"\nmsgstr[1] \"\"\n\n" ) sorted_pot_strings ; close_out fd @@ -948,7 +963,7 @@ let call_raw force_non_builtin_func (e, para) = (match para with | [ List(String([ s, List [] ], (_, pos_offset, _ as pos)) :: para) ] -> if !Flags.generate_pot then ( - Hashtbl.add pot_strings s (pos, !po_comments) ; + Hashtbl.add pot_strings (No_plural s) (pos, !po_comments) ; po_comments := [] ) ; let contexts = check_format_a_la_printf s pos_offset in @@ -962,6 +977,29 @@ let call_raw force_non_builtin_func (e, para) = | [ List(String _ :: _) ] -> die_rule "don't use interpolated translated string, use %s or %d instead" | _ -> die_rule (sprintf "%s() must be used with a string" f)) + | "P" -> + (match para with + | [ List(String([ s1, List [] ], (_, pos1_offset, _ as pos)) :: + String([ sn, List [] ], (_, posn_offset, _)) :: _n :: para) ] -> + if !Flags.generate_pot then ( + Hashtbl.add pot_strings (With_plural(s1, sn)) (pos, !po_comments) ; + po_comments := [] + ) ; + let contexts1 = check_format_a_la_printf s1 pos1_offset in + let contextsn = check_format_a_la_printf sn posn_offset in + if List.length contexts1 > List.length contextsn then + warn_rule [Warn_traps; Warn_MDK_Common] "the singular string must not use more parameters than the plural string" + else if contexts1 <> (take (List.length contexts1) contextsn) then + warn_rule [Warn_traps; Warn_MDK_Common] "the singular and plural strings do not use same parameters" + else if List.length para < List.length contextsn then + warn_rule [Warn_traps; Warn_MDK_Common] "not enough parameters" + else if List.length para > List.length contextsn then + warn_rule [Warn_traps; Warn_MDK_Common] "too many parameters" ; + (*if String.contains s '\t' then warn_rule "tabulation in translated string must be written \\\\t";*) + (*if count_matching_char s '\n' > 10 then warn_rule "long string";*) + | [ List(String _ :: _) ] -> die_rule "don't use interpolated translated string, use %s or %d instead" + | _ -> die_rule (sprintf "%s() must be used with a string" f)) + | "if_" -> (match para with | [ List [ _ ] ] -> warn_rule [Warn_traps; Warn_MDK_Common] "not enough parameters"; @@ -1187,8 +1225,13 @@ let rec mcontext2s = function | M_unknown -> "unknown" | M_mixed l -> String.concat " | " (List.map mcontext2s l) + | M_break_control_flow -> "break control flow" + let rec mcontext_lower c1 c2 = match c1, c2 with + | M_break_control_flow, _ -> false + | _, M_break_control_flow -> false + | M_special, _ | _, M_special -> internal_error "M_special in mcontext_compare" | M_unknown, _ @@ -1296,8 +1339,8 @@ let mcontext_check_unop_l wanted_mcontext esp = let mcontext_check_non_none esp = if esp.mcontext = M_none then warn_rule [Warn_context] "() context not accepted here" -let mcontext_check_none msg expr esp = - let rec mcontext_check_none_rec msg expr = function +let rec mcontext_check_none_rec msg expr = function + | M_break_control_flow -> () | M_none | M_unknown -> () | M_mixed l when List.exists (fun c -> c = M_none) l -> () | M_tuple l -> @@ -1318,10 +1361,28 @@ let mcontext_check_none msg expr esp = | [List [Num("1", _)]; Semi_colon] -> () (* allow "1;" for package return value. It would be much better to check we are at toplevel, but hell i don't want to wire this information up to here *) | [List [Call_op ("<>", [Ident (None, "STDIN", _)], _)]; Semi_colon] -> () (* allow to ask "press return" *) | [List [Call(Deref(I_func, Ident(None, "map", _)), _)]; Semi_colon] -> warn_rule [Warn_void] "if you don't use the return value, use \"foreach\" instead of \"map\"" - | _ -> warn [Warn_void] esp.pos msg - in + | _ -> warn_rule [Warn_void] msg + +let mcontext_check_none msg expr esp = mcontext_check_none_rec msg expr esp.mcontext +let mcontext_check_none_no_drop expr esp = + mcontext_check_none "value is dropped" expr esp + +let mcontext_check_none_should_drop expr esp = + mcontext_check_none "value should be dropped" expr esp + +let mcontext_check_none_no_drop_always esp1 esp_next = + match esp1.mcontext with + | M_break_control_flow -> + let not_Sub_declaration = function Sub_declaration _ -> false | _ -> true in + let l = List.filter not_Sub_declaration (fst esp_next.any) in + (match l with + | Label _ :: _ -> () (* that's ok, we have something like "... goto foo; ... return; foo: ..." *) + | [] -> () (* there are only sub declarations *) + | _ -> warn [Warn_traps] esp_next.pos "unreachable code") + | _ -> mcontext_check_none_no_drop esp1.any esp1 + (* only returns M_float when there is at least one float *) let mcontext_float_or_int l = List.iter (mcontext_check_raw M_float) l; @@ -1365,7 +1426,7 @@ let call_op_if_infix left right esp_start esp_end = warn_rule [Warn_suggest_simpler] "\"$foo = ... if !$foo\" can be written \"$foo ||= ...\"" | _ -> ()); - mcontext_check_none "value is dropped" [left] esp_start; + mcontext_check_none_no_drop [left] esp_start; (match right with | List [ Num("0", _)] -> () (* allow my $x if 0 *) | _ -> check_My_under_condition "replace \"my $foo = ... if \" with \"my $foo = && ...\"" left); @@ -1386,7 +1447,7 @@ let call_op_unless_infix left right esp_start esp_end = | _ -> ()); | _ -> ()); - mcontext_check_none "value is dropped" [left] esp_start; + mcontext_check_none_no_drop [left] esp_start; check_My_under_condition "replace \"my $foo = ... unless \" with \"my $foo = ! && ...\"" left; let pos = raw_pos_range esp_start esp_end in diff --git a/src/parser_helper.mli b/src/parser_helper.mli index e820703..38afaea 100644 --- a/src/parser_helper.mli +++ b/src/parser_helper.mli @@ -203,7 +203,6 @@ val to_Call_assign_op_ : Types.fromparser Types.prio_anyexpr Types.any_spaces_pos val followed_by_comma : Types.fromparser list -> bool -> Types.fromparser list -val pot_strings : (string, (string * int * int) * string list) Hashtbl.t val po_comments : string list ref val po_comment : string Types.any_spaces_pos -> unit val check_format_a_la_printf : string -> int -> Types.maybe_context list @@ -284,8 +283,17 @@ val mcontext_check_unop_l : Types.maybe_context -> Types.fromparser list Types.prio_anyexpr Types.any_spaces_pos -> unit val mcontext_check_non_none : 'a Types.any_spaces_pos -> unit +val mcontext_check_none_rec : + string -> Types.fromparser list -> Types.maybe_context -> unit val mcontext_check_none : string -> Types.fromparser list -> 'a Types.any_spaces_pos -> unit +val mcontext_check_none_no_drop : + Types.fromparser list -> 'a Types.any_spaces_pos -> unit +val mcontext_check_none_should_drop : + Types.fromparser list -> 'a Types.any_spaces_pos -> unit +val mcontext_check_none_no_drop_always : + Types.fromparser list Types.any_spaces_pos -> + (Types.fromparser list * 'a) Types.any_spaces_pos -> unit val mcontext_float_or_int : Types.maybe_context list -> Types.maybe_context val mcontext_op_assign : 'a Types.any_spaces_pos -> diff --git a/src/perl_checker.html.pl b/src/perl_checker.html.pl index e90d2eb..38ec959 100644 --- a/src/perl_checker.html.pl +++ b/src/perl_checker.html.pl @@ -1,5 +1,42 @@ $s = <<'EOF'; -perl_checker + + perl_checker + + + + +

Quick Start

+ +To use perl_checker, simply use "perl_checker a_file.pl" +

+To use under emacs, simply add the following line to your .emacs, +then when you visit a perl file, you can use Ctrl-Return to run perl_checker +on this file + +

+  (global-set-key [(control return)] (lambda () (interactive) (save-some-buffers 1) (compile (concat "perl_checker --restrict-to-files " (buffer-file-name (current-buffer))))))
+
+ +

+To use with vim, use something like: +

+  perl_checker --restrict-to-files scanner.pm > errors.err ; vim -c ':copen 4' -c ':so /usr/share/vim/ftplugin/perl_checker.vim' -q
+
+where /usr/share/vim/ftplugin/perl_checker.vim is + +
+" Error formats
+setlocal efm=
+  \%EFile\ \"%f\"\\,\ line\ %l\\,\ characters\ %c-%*\\d:,
+  \%EFile\ \"%f\"\\,\ line\ %l\\,\ character\ %c:%m,
+  \%+EReference\ to\ unbound\ regexp\ name\ %m,
+  \%Eocamlyacc:\ e\ -\ line\ %l\ of\ \"%f\"\\,\ %m,
+  \%Wocamlyacc:\ w\ -\ %m,
+  \%-Zmake%.%#,
+  \%C%m
+
+ +

Goals of perl_checker

    @@ -23,12 +60,21 @@ $s = <<'EOF'; (NB: the subset is chosen to keep a good expressivity)
-

Compared to Perl-Critic +

Compared to PPI and Perl-Critic

    -
  • perl_checker use its own OCaml-written perl parser, which is in no way as robust as PPI. - A PPI require is to be able to parse non finished perl documents. - perl_checker is a checker, and it is not a big deal to die horribly on a weird perl expression, telling the programmer what to write instead. +
  • perl_checker use its own OCaml-written parser. + This parser only handle a subset of perl, + whereas one of PPI's goal is to be able to parse non finished perl documents. +

    perl_checker is a checker: it is not a big deal to die horribly on a weird perl expression, it tells the programmer what to write instead. + The issue is that perl_checker includes inter-modules analysis, and it implies being able to parse non-perl_checker compliant modules. + A solution for this is perl_checker fake modules. No perfect solution though. + +

  • PPI doesn't handle operator priorities: 1 + 2 << 3 is parsed as +
    • PPI: a list [ Number(1), Operator(+), Number(2), Operator(<<), Number(3) ] +
    • perl_checker: a tree Operator(<<, [ Operator(+, [ Number(1), Number(2) ]), Number(3) ]) +
    + This limits perlcritic checks to a syntax level.
  • perl_checker is much faster (more than 100 times) (ML pattern matching rulez) @@ -39,24 +85,33 @@ $s = <<'EOF';

    Get it

    -CVS source +tarball +
    +SVN source +
    +MDK::Common tarball

    Implemented features

    -
    white space normalization -
    enforce a similar coding style. In many languages you can find a coding - style document (eg: the GNU one). - TESTS=force_layout.t +
    detect some Perl traps +
    some Perl expressions are stupid, and one gets a warning when running + them with perl -w. The drawback of perl -w is the lack of + code coverage, it only detects expressions which are evaluated. + + TESTS=various_errors.t
    -
    disallow complex expressions -
    perl_checker try to ban some weird-not-used-a-lot features. - TESTS=syntax_restrictions.t +
    context checks +
    Perl has types associated with variables names, the so-called "context". + Some expressions mixing contexts are stupid, perl_checker detects them. + + TESTS=context.t
    +
    suggest simpler expressions
    when there is a simpler way to write an expression, suggest it. It can also help detecting errors. @@ -64,13 +119,7 @@ $s = <<'EOF'; TESTS=suggest_better.t
    -
    context checks -
    Perl has types associated with variables names, the so-called "context". - Some expressions mixing contexts are stupid, perl_checker detects them. - TESTS=context.t - -
    function call check
    detection of unknown functions or mismatching prototypes (warning: since perl is a dynamic language, some spurious warnings may occur when a function @@ -79,6 +128,7 @@ $s = <<'EOF'; TESTS=prototype.t
    +
    method call check
    detection of unknown methods or mismatching prototypes. perl_checker doesn't have any idea what the object type is, it simply checks if a method @@ -87,19 +137,31 @@ $s = <<'EOF'; TESTS=method.t
    +
    return value check
    dropping the result of a functionnally pure function is stupid. using the result of a function returning void is stupid too. +
    (nb: perl_checker enforces && and || are used as boolean operators + whereas and and or are used for control flow) TESTS=return_value.t
    -
    detect some Perl traps -
    some Perl expressions are stupid, and one gets a warning when running - them with perl -w. The drawback are perl -w is the lack of - code coverage, it only detects expressions which are evaluated. - TESTS=various_errors.t +
    white space normalization +
    enforce a similar coding style. In many languages you can find a coding + style document (eg: the GNU one). + + TESTS=force_layout.t + +
    + +
    disallow complex expressions +
    perl_checker try to ban some weird-not-used-a-lot features. + + TESTS=syntax_restrictions.t + +
    @@ -136,7 +198,7 @@ sub get_example { join('', map { my $lines = join("
    ", map { "" . html_quote($_) . "" } @{$_->{lines}}); my $logs = join("
    ", map { html_quote($_) } @{$_->{logs}}); - " \n", $lines, "", $logs, "\n"; + $logs ? " \n" . $lines . "" . $logs . "\n" : ''; } @tests) . "\n"; } diff --git a/src/test/return_value.t b/src/test/return_value.t index b4786f5..89cf9ee 100644 --- a/src/test/return_value.t +++ b/src/test/return_value.t @@ -1,3 +1,11 @@ +die; xxx(); unreachable code + +exit 1; xxx(); unreachable code + +$xxx or die; + +next if $xxx; + if ($xxx or $yyy) {} value should be dropped context () is not compatible with context bool diff --git a/src/test/suggest_better.t b/src/test/suggest_better.t index d76abeb..208b7cc 100644 --- a/src/test/suggest_better.t +++ b/src/test/suggest_better.t @@ -12,6 +12,8 @@ $xxx->{yyy}->{zzz} the arrow "->" is unneeded "xxx\"$xxx" you can replace "xxx\"xxx" with qq(xxx"xxx), that way you don't need to escape <"> +"xxx\"xxx'" you can replace "xxx\"xxx" with qq(xxx"xxx), that way you don't need to escape <"> + /xxx\'xxx/ you can replace \' with ' /xxx\;xxx/ you can replace \; with ; diff --git a/src/test/various_errors.t b/src/test/various_errors.t index 48a8ece..3a4f4dd 100644 --- a/src/test/various_errors.t +++ b/src/test/various_errors.t @@ -1,5 +1,9 @@ local $xxx ||= $yyy applying ||= on a new initialized variable is wrong +xxx(!my $xxx) applying not on a new initialized variable is wrong + +xxx(!our $xxx) + $1 =~ s/xxx/yyy/ do not modify the result of a match (eg: $1) $xxx[1, 2] you must give only one argument diff --git a/src/tree.ml b/src/tree.ml index 16fd0e4..f21b9c1 100644 --- a/src/tree.ml +++ b/src/tree.ml @@ -188,7 +188,7 @@ let get_uses t = uses | Use(Ident(None, "base", _), classes) -> let l = List.map (fun (pkg, pos) -> (pkg, (None, pos))) (collect from_qw_raw classes) in - l @ uses + List.filter (fun (pkg, _) -> not (uses_external_package pkg)) l @ uses | Use(Ident(_, _, pos) as pkg, l) -> let package = string_of_fromparser pkg in if uses_external_package package then @@ -204,18 +204,20 @@ let get_uses t = ) [] t let get_isa t = + let get_isa_ isa exporter pos classes = + if isa <> None || exporter <> None then die_with_pos pos "\"use base\" and \"@ISA\" must be used once only"; + let special, l = List.partition (fun (s, _) -> s = "DynaLoader" || s = "Exporter") classes in + let exporter = if List.mem_assoc "Exporter" special then Some pos else None in + let isa = if l = [] && special <> [] then None else Some l in + isa, exporter + in List.fold_left (fun (isa, exporter) e -> match e with | Use(Ident(None, "base", pos), classes) -> - if isa <> None || exporter <> None then die_with_pos pos "\"use base\" and \"@ISA\" must be used once only"; - Some (collect from_qw_raw classes), None + get_isa_ isa exporter pos (collect from_qw_raw classes) | List [ Call_op("=", [ Deref(I_array, Ident(None, "ISA", pos)) ; classes ], _) ] | List [ Call_op("=", [ My_our("our", [ I_array, "ISA" ], pos) ; classes ], _) ] -> - if isa <> None || exporter <> None then die_with_pos pos "\"use base\" and \"@ISA\" must be used once only"; - let special, l = List.partition (fun (s, _) -> s = "DynaLoader" || s = "Exporter") (from_qw_raw classes) in - let exporter = if List.mem_assoc "Exporter" special then Some pos else None in - let isa = if l = [] && special <> [] then None else Some l in - isa, exporter + get_isa_ isa exporter pos (from_qw_raw classes) | _ -> isa, exporter ) (None, None) t @@ -308,6 +310,7 @@ let get_vars_declaration global_vars_declared file_name package = | Sub_declaration(Ident(Some fq, name, pos), perl_proto, body, _) -> Hashtbl.replace global_vars_declared (I_func, fq, name) (pos, get_proto perl_proto body) + | List [ Call_op("=", [My_our("our", ours, pos); _], _) ] | List [ Call_op("=", [My_our("local", ([ I_scalar, "_" ] as ours), pos); _], _) ] | List [ My_our("our", ours, pos) ] diff --git a/src/types.mli b/src/types.mli index 5f23d3a..1497f26 100644 --- a/src/types.mli +++ b/src/types.mli @@ -34,6 +34,8 @@ type maybe_context = | M_unknown | M_mixed of maybe_context list + | M_break_control_flow + type sub_declaration_kind = Real_sub_declaration | Glob_assign type fromparser = -- cgit v1.2.1