Support general warnings and errors in lint pass via flags and attrs. Close rust-lang#1543.

Author: graydon

src/librustsyntax/ext/expand.rs

@@ -76,12 +76,7 @@ fn expand_mod_items(exts: hashmap<str, syntax_extension>, cx: ext_ctxt,
               ast::meta_list(n, _) { n }
             };
             alt exts.find(mname) {
-              none { items }
-
-              some(normal(_)) | some(macro_defining(_)) {
-                cx.span_err(
-                    attr.span,
-                    #fmt["%s cannot be used as a decorator", mname]);
+              none | some(normal(_)) | some(macro_defining(_)) {
                 items
               }
 

src/rustc/driver/driver.rs

@@ -366,10 +366,16 @@ fn build_session_options(match: getopts::match,
 
     let parse_only = opt_present(match, "parse-only");
     let no_trans = opt_present(match, "no-trans");
-    let mut lint_opts = [];
-    if opt_present(match, "no-lint-ctypes") {
-        lint_opts += [(lint::ctypes, false)];
-    }
+
+    let lint_flags = (getopts::opt_strs(match, "W")
+                      + getopts::opt_strs(match, "warn"));
+    let lint_dict = lint::get_lint_dict();
+    let lint_opts = vec::map(lint_flags) {|flag|
+        alt lint::lookup_lint(lint_dict, flag) {
+          none { early_error(demitter, #fmt("unknown warning: %s", flag)) }
+          some(x) { x }
+        }
+    };
 
     let output_type =
         if parse_only || no_trans {
@@ -426,7 +432,6 @@ fn build_session_options(match: getopts::match,
     let addl_lib_search_paths = getopts::opt_strs(match, "L");
     let cfg = parse_cfgspecs(getopts::opt_strs(match, "cfg"));
     let test = opt_present(match, "test");
-    let warn_unused_imports = opt_present(match, "warn-unused-imports");
     let sopts: @session::options =
         @{crate_type: crate_type,
           static: static,
@@ -448,8 +453,7 @@ fn build_session_options(match: getopts::match,
           test: test,
           parse_only: parse_only,
           no_trans: no_trans,
-          no_asm_comments: no_asm_comments,
-          warn_unused_imports: warn_unused_imports};
+          no_asm_comments: no_asm_comments};
     ret sopts;
 }
 
@@ -521,11 +525,12 @@ fn opts() -> [getopts::opt] {
          optflag("time-passes"), optflag("time-llvm-passes"),
          optflag("count-llvm-insns"),
          optflag("no-verify"),
-         optflag("no-lint-ctypes"),
+
+         optmulti("W"), optmulti("warn"),
+
          optmulti("cfg"), optflag("test"),
          optflag("lib"), optflag("bin"), optflag("static"), optflag("gc"),
          optflag("no-asm-comments"),
-         optflag("warn-unused-imports"),
          optflag("enforce-mut-vars")];
 }
 

src/rustc/driver/rustc.rs

@@ -39,7 +39,6 @@ Options:
     --lib              Compile a library crate
     --ls               List the symbols defined by a compiled library crate
     --no-asm-comments  Do not add comments into the assembly source
-    --no-lint-ctypes   Suppress warnings for possibly incorrect ctype usage
     --no-trans         Run all passes except translation; no output
     --no-verify        Suppress LLVM verification step (slight speedup)
                        (see http://llvm.org/docs/Passes.html for detail)
@@ -65,13 +64,15 @@ Options:
                        (see http://sources.redhat.com/autobook/autobook/
                        autobook_17.html for detail)
 
+    -W <foo>           enable warning <foo>
+    -W no-<foo>        disable warning <foo>
+    -W err-<foo>       enable warning <foo> as an error
+
     --time-passes      Time the individual phases of the compiler
     --time-llvm-passes Time the individual phases of the LLVM backend
     --count-llvm-insns Count and categorize generated LLVM instructions
-    -v --version       Print version info and exit
-    --warn-unused-imports
-                       Warn about unnecessary imports
 
+    -v --version       Print version info and exit
 ");
 }
 

src/rustc/driver/session.rs

@@ -31,7 +31,7 @@ type options =
      debuginfo: bool,
      extra_debuginfo: bool,
      verify: bool,
-     lint_opts: [(lint::option, bool)],
+     lint_opts: [(lint::lint, lint::level)],
      save_temps: bool,
      stats: bool,
      time_passes: bool,
@@ -45,8 +45,7 @@ type options =
      test: bool,
      parse_only: bool,
      no_trans: bool,
-     no_asm_comments: bool,
-     warn_unused_imports: bool};
+     no_asm_comments: bool};
 
 type crate_metadata = {name: str, data: [u8]};
 

src/rustc/middle/lint.rs

@@ -1,117 +1,197 @@
 import driver::session::session;
-import middle::ty::ctxt;
+import middle::ty;
 import syntax::{ast, visit};
 import syntax::attr;
-import std::map::hashmap;
+import syntax::codemap::span;
+import std::map::{map,hashmap,hash_from_strs};
 import io::writer_util;
 
-enum option {
+export lint, ctypes, unused_imports;
+export level, ignore, warn, error;
+export lookup_lint, lint_dict, get_lint_dict, check_crate;
+
+#[doc="
+
+A 'lint' check is a kind of miscallaneous constraint that a user _might_ want
+to enforce, but might reasonably want to permit as well, on a module-by-module
+basis. They contrast with static constraints enforced by other phases of the
+compiler, which are generally required to hold in order to compile the program
+correctly at all.
+
+"]
+
+enum lint {
     ctypes,
+    unused_imports,
 }
 
-impl opt_ for option {
-    fn desc() -> str {
-        "lint: " + alt self {
-          ctypes { "ctypes usage checking" }
-        }
-    }
-    fn run(tcx: ty::ctxt, crate: @ast::crate, time_pass: bool) {
-        let checker = alt self {
-          ctypes {
-            bind check_ctypes(tcx, crate)
-          }
-        };
-        time(time_pass, self.desc(), checker);
-    }
+enum level {
+    ignore, warn, error
 }
 
-// FIXME: Copied from driver.rs, to work around a bug(#1566)
-fn time(do_it: bool, what: str, thunk: fn()) {
-    if !do_it{ ret thunk(); }
-    let start = std::time::precise_time_s();
-    thunk();
-    let end = std::time::precise_time_s();
-    io::stdout().write_str(#fmt("time: %3.3f s\t%s\n",
-                                end - start, what));
+type lint_spec = @{lint: lint,
+                   desc: str,
+                   default: level};
+
+type lint_dict = hashmap<str,lint_spec>;
+
+fn get_lint_dict() -> lint_dict {
+    let v = [
+        ("ctypes",
+         @{lint: ctypes,
+           desc: "proper use of core::libc types in native modules",
+           default: warn}),
+
+        ("unused-imports",
+         @{lint: unused_imports,
+           desc: "imports that are never used",
+           default: ignore})
+    ];
+    hash_from_strs(v)
 }
 
-// Merge lint options specified by crate attributes and rustc command
-// line. Precedence: cmdline > attribute > default
-fn merge_opts(attrs: [ast::attribute], cmd_opts: [(option, bool)]) ->
-    [(option, bool)] {
-    fn str_to_option(name: str) -> (option, bool) {
-        ret alt check name {
-          "ctypes" { (ctypes, true) }
-          "no_ctypes" { (ctypes, false) }
-        }
-    }
+type ctxt = @{dict: lint_dict,
+              curr: hashmap<lint, level>,
+              tcx: ty::ctxt};
 
-    fn meta_to_option(meta: @ast::meta_item) -> (option, bool) {
-        ret alt meta.node {
-          ast::meta_word(name) {
-            str_to_option(name)
-          }
-          _ { fail "meta_to_option: meta_list contains a non-meta-word"; }
-        };
+impl methods for ctxt {
+    fn get_level(lint: lint) -> level {
+        alt self.curr.find(lint) {
+          some(c) { c }
+          none { ignore }
+        }
     }
 
-    fn default() -> [(option, bool)] {
-        [(ctypes, true)]
+    fn set_level(lint: lint, level: level) {
+        if level == ignore {
+            self.curr.remove(lint);
+        } else {
+            self.curr.insert(lint, level);
+        }
     }
 
-    fn contains(xs: [(option, bool)], x: option) -> bool {
-        for xs.each {|c|
-            let (o, _) = c;
-            if o == x { ret true; }
+    fn span_lint(level: level, span: span, msg: str) {
+        alt level {
+          ignore { }
+          warn { self.tcx.sess.span_warn(span, msg); }
+          error { self.tcx.sess.span_err(span, msg); }
         }
-        ret false;
     }
 
-    let mut result = cmd_opts;
+    #[doc="
+          Merge the warnings specified by any `warn(...)` attributes into the
+          current lint context, call the provided function, then reset the
+          warnings in effect to their previous state.
+    "]
+    fn with_warn_attrs(attrs: [ast::attribute], f: fn(ctxt)) {
 
-    let lint_metas =
-        attr::attr_metas(attr::find_attrs_by_name(attrs, "lint"));
+        let mut undo = [];
 
-    vec::iter(lint_metas) {|mi|
-        alt mi.node {
-          ast::meta_list(_, list) {
-            vec::iter(list) {|e|
-                let (o, v) = meta_to_option(e);
-                if !contains(cmd_opts, o) {
-                    result += [(o, v)];
+        let metas = attr::attr_metas(attr::find_attrs_by_name(attrs, "warn"));
+        for metas.each {|meta|
+            alt meta.node {
+              ast::meta_list(_, metas) {
+                for metas.each {|meta|
+                    alt meta.node {
+                      ast::meta_word(lintname) {
+                        alt lookup_lint(self.dict, lintname) {
+                          none {
+                            self.tcx.sess.span_err(
+                                meta.span,
+                                #fmt("unknown warning: '%s'", lintname));
+                          }
+                          some((lint, new_level)) {
+                            let old_level = self.get_level(lint);
+                            self.set_level(lint, new_level);
+                            undo += [(lint, old_level)]
+                          }
+                        }
+                      }
+                      _ {
+                        self.tcx.sess.span_err(
+                            meta.span,
+                            "malformed warning attribute");
+                      }
+                    }
                 }
+              }
+              _ {
+                self.tcx.sess.span_err(meta.span,
+                                       "malformed warning attribute");
+              }
             }
-          }
-          _ { }
         }
+
+        f(self);
+
+        for undo.each {|pair|
+            let (lint,old_level) = pair;
+            self.set_level(lint, old_level);
+        }
+    }
+}
+
+
+fn lookup_lint(dict: lint_dict, s: str)
+    -> option<(lint, level)> {
+    let s = str::replace(s, "-", "_");
+    let (name, level) = if s.starts_with("no_") {
+        (s.substr(3u, s.len() - 3u), ignore)
+    } else if s.starts_with("err_") {
+        (s.substr(4u, s.len() - 4u), error)
+    } else {
+        (s, warn)
     };
+    alt dict.find(name) {
+      none { none }
+      some(spec) { some((spec.lint, level)) }
+    }
+}
+
+
+// FIXME: Copied from driver.rs, to work around a bug(#1566)
+fn time(do_it: bool, what: str, thunk: fn()) {
+    if !do_it{ ret thunk(); }
+    let start = std::time::precise_time_s();
+    thunk();
+    let end = std::time::precise_time_s();
+    io::stdout().write_str(#fmt("time: %3.3f s\t%s\n",
+                                end - start, what));
+}
 
-    for default().each {|c|
-        let (o, v) = c;
-        if !contains(result, o) {
-            result += [(o, v)];
+fn check_item(cx: ctxt, i: @ast::item) {
+    cx.with_warn_attrs(i.attrs) {|cx|
+        cx.curr.items {|lint, level|
+            alt lint {
+              ctypes { check_item_ctypes(cx, level, i); }
+              unused_imports { check_item_unused_imports(cx, level, i); }
+            }
         }
     }
+}
 
-    ret result;
+fn check_item_unused_imports(_cx: ctxt, _level: level, _it: @ast::item) {
+    // FIXME: Don't know how to check this in lint yet, it's currently being
+    // done over in resolve. When resolve is rewritten, do it here instead.
 }
 
-fn check_ctypes(tcx: ty::ctxt, crate: @ast::crate) {
-    fn check_native_fn(tcx: ty::ctxt, decl: ast::fn_decl) {
+fn check_item_ctypes(cx: ctxt, level: level, it: @ast::item) {
+
+    fn check_native_fn(cx: ctxt, level: level, decl: ast::fn_decl) {
         let tys = vec::map(decl.inputs) {|a| a.ty };
         for vec::each(tys + [decl.output]) {|ty|
             alt ty.node {
               ast::ty_path(_, id) {
-                alt tcx.def_map.get(id) {
+                alt cx.tcx.def_map.get(id) {
                   ast::def_prim_ty(ast::ty_int(ast::ty_i)) {
-                    tcx.sess.span_warn(
-                        ty.span,
+                    cx.span_lint(
+                        level, ty.span,
                         "found rust type `int` in native module, while \
                          libc::c_int or libc::c_long should be used");
                   }
                   ast::def_prim_ty(ast::ty_uint(ast::ty_u)) {
-                    tcx.sess.span_warn(
-                        ty.span,
+                    cx.span_lint(
+                        level, ty.span,
                         "found rust type `uint` in native module, while \
                          libc::c_uint or libc::c_ulong should be used");
                   }
@@ -123,40 +203,55 @@ fn check_ctypes(tcx: ty::ctxt, crate: @ast::crate) {
         }
     }
 
-    fn check_item(tcx: ty::ctxt, it: @ast::item) {
-        alt it.node {
-          ast::item_native_mod(nmod) if attr::native_abi(it.attrs) !=
-              either::right(ast::native_abi_rust_intrinsic) {
-            for nmod.items.each {|ni|
-                alt ni.node {
-                  ast::native_item_fn(decl, tps) {
-                    check_native_fn(tcx, decl);
-                  }
-                  _ { }
-                }
+    alt it.node {
+      ast::item_native_mod(nmod) if attr::native_abi(it.attrs) !=
+      either::right(ast::native_abi_rust_intrinsic) {
+        for nmod.items.each {|ni|
+            alt ni.node {
+              ast::native_item_fn(decl, tps) {
+                check_native_fn(cx, level, decl);
+              }
+              _ { }
             }
-          }
-          _ {/* nothing to do */ }
         }
+      }
+      _ {/* nothing to do */ }
     }
-
-    let visit = visit::mk_simple_visitor(@{
-        visit_item: bind check_item(tcx, _)
-        with *visit::default_simple_visitor()
-    });
-    visit::visit_crate(*crate, (), visit);
 }
 
+
 fn check_crate(tcx: ty::ctxt, crate: @ast::crate,
-               opts: [(option, bool)], time: bool) {
-    let lint_opts = lint::merge_opts(crate.node.attrs, opts);
-    for lint_opts.each {|opt|
-        let (lopt, switch) = opt;
-        if switch == true {
-            lopt.run(tcx, crate, time);
+               lint_opts: [(lint, level)], time_pass: bool) {
+
+    fn hash_lint(&&lint: lint) -> uint { lint as uint }
+    fn eq_lint(&&a: lint, &&b: lint) -> bool { a == b }
+
+    let cx = @{dict: get_lint_dict(),
+               curr: hashmap(hash_lint, eq_lint),
+               tcx: tcx};
+
+    // Install defaults.
+    cx.dict.items {|_k, spec| cx.set_level(spec.lint, spec.default); }
+
+    // Install command-line options, overriding defaults.
+    for lint_opts.each {|pair|
+        let (lint,level) = pair;
+        cx.set_level(lint, level);
+    }
+
+    time(time_pass, "lint checking") {||
+        cx.with_warn_attrs(crate.node.attrs) {|cx|
+            let visit = visit::mk_simple_visitor(@{
+                visit_item: fn@(i: @ast::item) { check_item(cx, i); }
+                with *visit::default_simple_visitor()
+            });
+            visit::visit_crate(*crate, (), visit);
         }
     }
+
+    tcx.sess.abort_if_errors();
 }
+
 //
 // Local Variables:
 // mode: rust

src/rustc/middle/resolve.rs

@@ -158,9 +158,16 @@ fn resolve_crate(sess: session, amap: ast_map::map, crate: @ast::crate) ->
     // check_for_collisions must happen after resolve_names so we
     // don't complain if a pattern uses the same nullary enum twice
     check_for_collisions(e, *crate);
-    if sess.opts.warn_unused_imports {
-        check_unused_imports(e);
+
+    // FIXME: move this to the lint pass when rewriting resolve.
+    for sess.opts.lint_opts.each {|pair|
+        let (lint,level) = pair;
+        if lint == lint::unused_imports && level != lint::ignore {
+            check_unused_imports(e, level);
+            break;
+        }
     }
+
     ret {def_map: e.def_map, exp_map: e.exp_map, impl_map: e.impl_map};
 }
 
@@ -361,12 +368,24 @@ fn resolve_imports(e: env) {
     e.sess.abort_if_errors();
 }
 
-fn check_unused_imports(e: @env) {
+// FIXME (#1634): move this to the lint pass when rewriting resolve. It's
+// using lint-specific control flags presently but resolve-specific data
+// structures. Should use the general lint framework (with scopes, attrs).
+fn check_unused_imports(e: @env, level: lint::level) {
     e.imports.items {|k, v|
         alt v {
             resolved(_, _, _, _, name, sp) {
               if !vec::contains(e.used_imports.data, k) {
-                e.sess.span_warn(sp, "unused import " + name);
+                  alt level {
+                    lint::warn {
+                      e.sess.span_warn(sp, "unused import " + name);
+                    }
+                    lint::error {
+                      e.sess.span_err(sp, "unused import " + name);
+                    }
+                    lint::ignore {
+                    }
+                  }
               }
             }
             _ { }

src/rustdoc/astsrv.rs

@@ -148,7 +148,6 @@ fn build_session() -> (session::session, @mut bool) {
         parse_only: false,
         no_trans: false,
         no_asm_comments: false,
-        warn_unused_imports: false
     };
 
     let codemap = codemap::new_codemap();

src/test/compile-fail/unused-imports-warn.rs

@@ -1,5 +1,5 @@
 // error-pattern:unused import
-// compile-flags:--warn-unused-imports
+// compile-flags:-W unused-imports
 import cal = bar::c::cc;
 
 mod foo {

src/test/compile-fail/warn-ctypes-err-attr.rs

@@ -0,0 +1,10 @@
+// error-pattern:found rust type
+#[warn(err_ctypes)];
+
+#[nolink]
+native mod libc {
+    fn malloc(size: int) -> *u8;
+}
+
+fn main() {
+}

src/test/compile-fail/warn-ctypes.rs

@@ -0,0 +1,9 @@
+// compile-flags:-W err-ctypes
+// error-pattern:found rust type
+#[nolink]
+native mod libc {
+    fn malloc(size: int) -> *u8;
+}
+
+fn main() {
+}

src/test/run-pass/warn-ctypes-inhibit.rs

@@ -0,0 +1,11 @@
+// compile-flags:-W err-ctypes
+
+#[warn(no_ctypes)];
+
+#[nolink]
+native mod libc {
+    fn malloc(size: int) -> *u8;
+}
+
+fn main() {
+}