re-sync after the big svn loss1.2.41.2.3

13 files changed, 215 insertions, 64 deletions

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 <STDIN> 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 <cond>\" with \"my $foo = <cond> && ...\"" 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 <cond>\" with \"my $foo = !<cond> && ...\"" 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';
-<head><title>perl_checker</title></head>
+<head>
+  <title>perl_checker</title>
+  <style> body { max-width: 900; } </style>
+</head>
+
+
+<h1>Quick Start</h1>
+
+To use perl_checker, simply use "perl_checker a_file.pl"
+<p>
+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
+
+<pre>
+  (global-set-key [(control return)] (lambda () (interactive) (save-some-buffers 1) (compile (concat "perl_checker --restrict-to-files " (buffer-file-name (current-buffer))))))
+</pre>
+
+<p>
+To use with vim, use something like:
+<pre>
+  perl_checker --restrict-to-files scanner.pm > errors.err ; vim -c ':copen 4' -c ':so /usr/share/vim/ftplugin/perl_checker.vim' -q
+</pre>
+where /usr/share/vim/ftplugin/perl_checker.vim is
+
+<pre>
+" 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
+</pre>
+
+
 <h1>Goals of perl_checker</h1>
 
 <ul>
@@ -23,12 +60,21 @@ $s = <<'EOF';
      (NB: the subset is chosen to keep a good expressivity)
 </ul>
 
-<h1>Compared to <a href="http://perlcritic.tigris.org/">Perl-Critic</a>
+<h1>Compared to <a href="http://www.perl.com/pub/a/2005/06/09/ppi.html">PPI</a> and <a href="http://perlcritic.tigris.org/">Perl-Critic</a></h1>
 
 <ul>
-<li>perl_checker use its own OCaml-written perl parser, which is in no way as robust as <a href="http://www.perl.com/pub/a/2005/06/09/ppi.html">PPI</a>.
-    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.
+<li>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.
+  <p>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 <i>fake</i> modules. No perfect solution though.
+
+<li>PPI doesn't handle operator priorities: <tt>1 + 2 &lt;&lt; 3</tt> is parsed as
+    <ul><li>PPI: a list [ Number(<tt>1</tt>), Operator(<tt>+</tt>), Number(<tt>2</tt>), Operator(<tt>&lt;&lt;</tt>), Number(<tt>3</tt>) ]
+        <li>perl_checker: a tree Operator(<tt>&lt;&lt;</tt>, [ Operator(<tt>+</tt>, [ Number(<tt>1</tt>), Number(<tt>2</tt>) ]), Number(<tt>3</tt>) ])
+    </ul>
+    This limits perlcritic checks to a syntax level.
 
 <li>perl_checker is <b>much</b> faster (more than 100 times) (ML pattern matching rulez)
 
@@ -39,24 +85,33 @@ $s = <<'EOF';
 
 <h1>Get it</h1>
 
-<a href="http://cvs.mandriva.com/cgi-bin/cvsweb.cgi/soft/perl-MDK-Common/perl_checker.src/">CVS source</a>
+<a href="http://svn.mandriva.com/cgi-bin/viewvc.cgi/packages/cooker/perl_checker/current/SOURCES/">tarball</a>
+<br>
+<a href="http://svn.mandriva.com/cgi-bin/viewvc.cgi/soft/perl_checker/">SVN source</a>
+<br>
+<a href="http://svn.mandriva.com/cgi-bin/viewvc.cgi/packages/cooker/perl-MDK-Common/current/SOURCES/">MDK::Common tarball</a>
 
 <h1>Implemented features</h1>
 
 <dl>
- <dt>white space normalization
- <dd>enforce a similar coding style. In many languages you can find a coding
- style document (eg: <a href="http://www.gnu.org/prep/standards_23.html">the GNU one</a>).
 
- TESTS=force_layout.t
+ <dt>detect some Perl traps
+ <dd>some Perl expressions are stupid, and one gets a warning when running
+ them with <tt>perl -w</tt>. The drawback of <tt>perl -w</tt> is the lack of
+ code coverage, it only detects expressions which are evaluated.
+
+ TESTS=various_errors.t
 
  </dd>
- <dt>disallow <i>complex</i> expressions
- <dd>perl_checker try to ban some weird-not-used-a-lot features.
 
- TESTS=syntax_restrictions.t
+ <dt>context checks
+ <dd>Perl has types associated with variables names, the so-called "context".
+ Some expressions mixing contexts are stupid, perl_checker detects them.
+
+ TESTS=context.t
 
  </dd>
+
  <dt>suggest simpler expressions
  <dd>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
 
  </dd>
- <dt>context checks
- <dd>Perl has types associated with variables names, the so-called "context".
- Some expressions mixing contexts are stupid, perl_checker detects them.
 
- TESTS=context.t
-
- </dd>
  <dt>function call check
  <dd>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
 
  </dd>
+
  <dt>method call check
  <dd>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
 
  </dd>
+
  <dt>return value check
  <dd>dropping the result of a functionnally <i>pure</i> function is stupid.
  using the result of a function returning void is stupid too.
+ <br>(nb: perl_checker enforces <tt>&&</tt> and <tt>||</tt> are used as boolean operators 
+      whereas <tt>and</tt> and <tt>or</tt> are used for control flow)
 
  TESTS=return_value.t
 
  </dd>
- <dt>detect some Perl traps
- <dd>some Perl expressions are stupid, and one gets a warning when running
- them with <tt>perl -w</tt>. The drawback are <tt>perl -w</tt> is the lack of
- code coverage, it only detects expressions which are evaluated.
 
- TESTS=various_errors.t
+ <dt>white space normalization
+ <dd>enforce a similar coding style. In many languages you can find a coding
+ style document (eg: <a href="http://www.gnu.org/prep/standards/standards.html#Writing-C">the GNU one</a>).
+
+ TESTS=force_layout.t
+
+ </dd>
+
+ <dt>disallow <i>complex</i> expressions
+ <dd>perl_checker try to ban some weird-not-used-a-lot features.
+
+ TESTS=syntax_restrictions.t
+
+ </dd>
 
 </dl>
 
@@ -136,7 +198,7 @@ sub get_example {
       join('', map { 
 	  my $lines = join("<br>", map { "<tt>" . html_quote($_) . "</tt>" } @{$_->{lines}});
 	  my $logs = join("<br>", map { html_quote($_) } @{$_->{logs}});
-	  "  <tr><td>\n", $lines, "</td><td>", $logs, "</td></tr>\n";
+	  $logs ? "  <tr><td>\n" . $lines . "</td><td>" . $logs . "</td></tr>\n" : '';
       } @tests) .
       "</table></a>\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 =