Add --warnings --exclude-warnings compiler options

* Add warnings checks for deprecated methods
* Support warning for build, run and spec commands
* Extract path handling operations of formatter to reuse them
 * Defaults set to --warnings=emit --exclude-warnings=libs


fixup! Add --warnings --exclude-warnings compiler options

etc/completion.bash

@@ -26,15 +26,15 @@ _crystal()
             ;;
         build)
             if [[ ${cur} == -* ]] ; then
-                local opts="--cross-compile --debug --emit --ll --link-flags --mcpu --no-color --no-codegen --prelude --release --single-module --threads --target --verbose --help"
+                local opts="--cross-compile --debug --emit --exclude-warnings --ll --link-flags --mcpu --no-color --no-codegen --prelude --release --single-module --threads --target --verbose --warnings --help"
                 COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) )
             else
                 COMPREPLY=($(_crystal_compgen_files $cur))
             fi
             ;;
         run)
             if [[ ${cur} == -* ]] ; then
-                local opts="--debug --define --emit --format --help --ll --link-flags --mcpu --no-color --no-codegen --prelude --release --stats --single-module --threads --verbose"
+                local opts="--debug --define --emit --exclude-warnings --format --help --ll --link-flags --mcpu --no-color --no-codegen --prelude --release --stats --single-module --threads --verbose --warnings"
                 COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) )
             else
                 COMPREPLY=($(_crystal_compgen_files $cur))

spec/compiler/codegen/warnings_spec.cr

@@ -0,0 +1,157 @@
+require "../spec_helper"
+
+describe "Code gen: warnings" do
+  it "detects top-level deprecated methods" do
+    assert_warning %(
+      @[Deprecated("Do not use me")]
+      def foo
+      end
+
+      foo
+    ), "Warning in line 6: Deprecated top-level foo. Do not use me",
+      inject_primitives: false
+  end
+
+  it "deprecation reason is optional" do
+    assert_warning %(
+      @[Deprecated]
+      def foo
+      end
+
+      foo
+    ), "Warning in line 6: Deprecated top-level foo.",
+      inject_primitives: false
+  end
+
+  it "detects deprecated instance methods" do
+    assert_warning %(
+      class Foo
+        @[Deprecated("Do not use me")]
+        def m
+        end
+      end
+
+      Foo.new.m
+    ), "Warning in line 8: Deprecated Foo#m. Do not use me",
+      inject_primitives: false
+  end
+
+  it "detects deprecated class methods" do
+    assert_warning %(
+      class Foo
+        @[Deprecated("Do not use me")]
+        def self.m
+        end
+      end
+
+      Foo.m
+    ), "Warning in line 8: Deprecated Foo.m. Do not use me",
+      inject_primitives: false
+  end
+
+  it "detects deprecated generic instance methods" do
+    assert_warning %(
+      class Foo(T)
+        @[Deprecated("Do not use me")]
+        def m
+        end
+      end
+
+      Foo(Int32).new.m
+    ), "Warning in line 8: Deprecated Foo(Int32)#m. Do not use me",
+      inject_primitives: false
+  end
+
+  it "detects deprecated generic class methods" do
+    assert_warning %(
+      class Foo(T)
+        @[Deprecated("Do not use me")]
+        def self.m
+        end
+      end
+
+      Foo(Int32).m
+    ), "Warning in line 8: Deprecated Foo(Int32).m. Do not use me",
+      inject_primitives: false
+  end
+
+  it "detects deprecated module methods" do
+    assert_warning %(
+      module Foo
+        @[Deprecated("Do not use me")]
+        def self.m
+        end
+      end
+
+      Foo.m
+    ), "Warning in line 8: Deprecated Foo.m. Do not use me",
+      inject_primitives: false
+  end
+
+  it "ignore deprecation excluded locations" do
+    with_tempfile("check_warnings_excludes") do |path|
+      FileUtils.mkdir_p File.join(path, "lib")
+
+      # NOTE tempfile might be created in symlinked folder
+      # which affects how to match current dir /var/folders/...
+      # with the real path /private/var/folders/...
+      path = File.real_path(path)
+
+      main_filename = File.join(path, "main.cr")
+      output_filename = File.join(path, "main")
+
+      Dir.cd(path) do
+        File.write main_filename, %(
+          require "./lib/foo"
+
+          bar
+          foo
+        )
+        File.write File.join(path, "lib", "foo.cr"), %(
+          @[Deprecated("Do not use me")]
+          def foo
+          end
+
+          def bar
+            foo
+          end
+        )
+
+        compiler = Compiler.new
+        compiler.warnings = Warnings::Emit
+        compiler.warnings_exclude << Crystal.normalize_path "lib"
+        compiler.prelude = "empty"
+        result = compiler.compile Compiler::Source.new(main_filename, File.read(main_filename)), output_filename
+
+        result.program.warning_failures.size.should eq(1)
+      end
+    end
+  end
+
+  it "errors if invalid argument type" do
+    assert_error %(
+      @[Deprecated(42)]
+      def foo
+      end
+      ),
+      "Error in line 3: first argument must be a String"
+  end
+
+  it "errors if too many arguments" do
+    assert_error %(
+      @[Deprecated("Do not use me", "extra arg")]
+      def foo
+      end
+      ),
+      "Error in line 3: wrong number of deprecated annotation arguments (given 2, expected 1)"
+  end
+
+  it "errors if missing link arguments" do
+    assert_error %(
+      @[Deprecated(invalid: "Do not use me")]
+      def foo
+      end
+      ),
+      "Error in line 3: too many named arguments (given 1, expected maximum 0)"
+  end
+end

spec/spec_helper.cr

@@ -108,6 +108,20 @@ def assert_error(str, message, inject_primitives = true)
   end
 end
 
+def assert_warning(code, message, inject_primitives = true)
+  code = inject_primitives(code) if inject_primitives
+
+  output_filename = Crystal.tempfile("crystal-spec-output")
+
+  compiler = Compiler.new
+  compiler.warnings = Warnings::Emit
+  compiler.prelude = "empty" # avoid issues in the current std lib
+  result = compiler.compile Compiler::Source.new("code.cr", code), output_filename
+
+  result.program.warning_failures.size.should eq(1)
+  result.program.warning_failures[0].should start_with(message)
+end
+
 def assert_macro(macro_args, macro_body, call_args, expected, expected_pragmas = nil, flags = nil)
   assert_macro(macro_args, macro_body, expected, expected_pragmas, flags) { call_args }
 end

src/compiler/crystal/codegen/call.cr

@@ -16,6 +16,8 @@ class Crystal::CodeGenVisitor
       return false
     end
 
+    check_call_to_deprecated_method node
+
     owner = node.name == "super" ? node.scope : node.target_def.owner
 
     call_args, has_out = prepare_call_args node, owner

src/compiler/crystal/codegen/warnings.cr

@@ -0,0 +1,93 @@
+module Crystal
+  struct DeprecatedAnnotation
+    getter message : String?
+
+    def initialize(@message = nil)
+    end
+
+    def self.from(ann : Annotation)
+      args = ann.args
+      named_args = ann.named_args
+
+      if named_args
+        ann.raise "too many named arguments (given #{named_args.size}, expected maximum 0)"
+      end
+
+      message = nil
+      count = 0
+
+      args.each do |arg|
+        case count
+        when 0
+          arg.raise "first argument must be a String" unless arg.is_a?(StringLiteral)
+          message = arg.value
+        else
+          ann.wrong_number_of "deprecated annotation arguments", args.size, "1"
+        end
+
+        count += 1
+      end
+
+      new(message)
+    end
+  end
+
+  class Def
+    def short_reference
+      case owner
+      when Program
+        "top-level #{name}"
+      when .metaclass?
+        "#{owner.instance_type}.#{name}"
+      else
+        "#{owner}##{name}"
+      end
+    end
+  end
+
+  class CodeGenVisitor
+    def check_call_to_deprecated_method(node : Call)
+      return unless @program.warnings.emit? || @program.warnings.error?
+
+      if (ann = node.target_def.annotation(@program.deprecated_annotation)) &&
+         (deprecated_annotation = DeprecatedAnnotation.from(ann))
+        return if ignore_warning_due_to_location(node.location)
+
+        message = deprecated_annotation.message
+        message = message ? " #{message}" : ""
+
+        full_message = node.warning "Deprecated #{node.target_def.short_reference}.#{message}"
+
+        @program.warning_failures << full_message
+      end
+    end
+
+    private def ignore_warning_due_to_location(location : Location?)
+      return false unless location
+
+      filename = location.original_filename
+      return false unless filename
+
+      return @program.warnings_exclude.any? do |path|
+        filename.starts_with?(path)
+      end
+    end
+  end
+
+  class Command
+    def report_warnings(result : Compiler::Result)
+      if (result.program.warnings.emit? || result.program.warnings.error?) &&
+         result.program.warning_failures.size > 0
+        result.program.warning_failures.each do |message|
+          STDERR.puts message
+        end
+
+        STDERR.puts "A total of #{result.program.warning_failures.size} warnings were found."
+      end
+    end
+
+    def warnings_fail_on_exit?(result : Compiler::Result)
+      result.program.warnings.error? && result.program.warning_failures.size > 0
+    end
+  end
+end

src/compiler/crystal/command.cr

@@ -64,7 +64,10 @@ class Crystal::Command
       init
     when "build".starts_with?(command)
       options.shift
-      build
+      result = build
+      report_warnings result
+      exit 1 if warnings_fail_on_exit?(result)
+      result
     when "play".starts_with?(command)
       options.shift
       playground
@@ -168,7 +171,9 @@ class Crystal::Command
   private def run_command(single_file = false)
     config = create_compiler "run", run: true, single_file: single_file
     if config.specified_output
-      config.compile
+      result = config.compile
+      report_warnings result
+      exit 1 if warnings_fail_on_exit?(result)
       return
     end
 
@@ -177,6 +182,9 @@ class Crystal::Command
     result = config.compile output_filename
 
     unless config.compiler.no_codegen?
+      report_warnings result
+      exit 1 if warnings_fail_on_exit?(result)
+
       execute output_filename, config.arguments, config.compiler
     end
   end
@@ -197,7 +205,7 @@ class Crystal::Command
     {config, result}
   end
 
-  private def execute(output_filename, run_args, compiler)
+  private def execute(output_filename, run_args, compiler, *, error_on_exit = false)
     time? = @time && !@progress_tracker.stats?
     status, elapsed_time = @progress_tracker.stage("Execute") do
       begin
@@ -226,7 +234,7 @@ class Crystal::Command
     end
 
     if status.normal_exit?
-      exit status.exit_code
+      exit error_on_exit ? 1 : status.exit_code
     else
       case status.exit_signal
       when ::Signal::KILL
@@ -353,6 +361,7 @@ class Crystal::Command
         opts.on("--mattr CPU", "Target specific features") do |features|
           compiler.mattr = features
         end
+        setup_compiler_warning_options(opts, compiler)
       end
 
       opts.on("--no-color", "Disable colored output") do
@@ -511,9 +520,30 @@ class Crystal::Command
       @color = false
       compiler.color = false
     end
+    setup_compiler_warning_options(opts, compiler)
     opts.invalid_option { }
   end
 
+  private def setup_compiler_warning_options(opts, compiler)
+    compiler.warnings_exclude << Crystal.normalize_path "lib"
+    opts.on("--warnings emit|error|ignore", "How to treat warnings. (default: emit)") do |w|
+      compiler.warnings = case w
+                          when "emit"
+                            Crystal::Warnings::Emit
+                          when "error"
+                            Crystal::Warnings::Error
+                          when "ignore"
+                            Crystal::Warnings::Ignore
+                          else
+                            error "--warnings should be emit, error, or ignore"
+                            raise "unreachable"
+                          end
+    end
+    opts.on("--exclude-warnings <path>", "Exclude warnings from path (default: lib)") do |f|
+      compiler.warnings_exclude << Crystal.normalize_path f
+    end
+  end
+
   private def validate_emit_values(values)
     emit_targets = Compiler::EmitTarget::None
     values.each do |value|