Decouple warning detection from program instances (#12293)

spec/compiler/semantic/warnings_spec.cr

@@ -227,12 +227,12 @@ describe "Semantic: warnings" do
             CR
 
           compiler = create_spec_compiler
-          compiler.warnings = Warnings::All
-          compiler.warnings_exclude << Crystal.normalize_path "lib"
+          compiler.warnings.level = :all
+          compiler.warnings.exclude_lib_path = true
           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)
+          compiler.warnings.infos.size.should eq(1)
         end
       end
     end
@@ -396,12 +396,12 @@ describe "Semantic: warnings" do
           )
 
           compiler = create_spec_compiler
-          compiler.warnings = Warnings::All
-          compiler.warnings_exclude << Crystal.normalize_path "lib"
+          compiler.warnings.level = :all
+          compiler.warnings.exclude_lib_path = true
           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)
+          compiler.warnings.infos.size.should eq(1)
         end
       end
     end

spec/spec_helper.cr

@@ -117,7 +117,7 @@ def assert_no_errors(*args, **opts)
 end
 
 def warnings_result(code)
-  semantic(code).program.warning_failures
+  semantic(code).program.warnings.infos
 end
 
 def assert_warning(code, message, *, file = __FILE__, line = __LINE__)

src/compiler/crystal/command.cr

@@ -592,30 +592,26 @@ class Crystal::Command
   end
 
   private def setup_compiler_warning_options(opts, compiler)
-    compiler.warnings_exclude << Crystal.normalize_path "lib"
-    warnings_exclude_default = true
-    opts.on("--warnings all|none", "Which warnings detect. (default: all)") do |w|
-      compiler.warnings = case w
-                          when "all"
-                            Crystal::Warnings::All
-                          when "none"
-                            Crystal::Warnings::None
-                          else
-                            error "--warnings should be all, or none"
-                            raise "unreachable"
-                          end
+    opts.on("--warnings all|none", "Which warnings to detect. (default: all)") do |w|
+      compiler.warnings.level = case w
+                                when "all"
+                                  Crystal::WarningLevel::All
+                                when "none"
+                                  Crystal::WarningLevel::None
+                                else
+                                  error "--warnings should be all, or none"
+                                  raise "unreachable"
+                                end
     end
     opts.on("--error-on-warnings", "Treat warnings as errors.") do |w|
-      compiler.error_on_warnings = true
+      compiler.warnings.error_on_warnings = true
     end
     opts.on("--exclude-warnings <path>", "Exclude warnings from path (default: lib)") do |f|
-      if warnings_exclude_default
-        # if an --exclude-warnings is used explicitly, then we remove the default value
-        compiler.warnings_exclude.clear
-        warnings_exclude_default = false
-      end
-      compiler.warnings_exclude << Crystal.normalize_path f
+      compiler.warnings.exclude_lib_path = false
+      compiler.warnings.exclude_path(f)
     end
+
+    compiler.warnings.exclude_lib_path = true
   end
 
   private def validate_emit_values(values)

src/compiler/crystal/compiler.cr

@@ -15,11 +15,6 @@ module Crystal
     Default     = LineNumbers
   end
 
-  enum Warnings
-    All
-    None
-  end
-
   # Main interface to the compiler.
   #
   # A Compiler parses source code, type checks it and
@@ -109,14 +104,8 @@ module Crystal
     # and can later be used to generate API docs.
     property? wants_doc = false
 
-    # Which kind of warnings wants to be detected.
-    property warnings : Warnings = Warnings::All
-
-    # Paths to ignore for warnings detection.
-    property warnings_exclude : Array(String) = [] of String
-
-    # If `true` compiler will error if warnings are found.
-    property error_on_warnings : Bool = false
+    # Warning settings and all detected warnings.
+    property warnings = WarningCollection.new
 
     @[Flags]
     enum EmitTarget
@@ -218,8 +207,6 @@ module Crystal
       program.show_error_trace = show_error_trace?
       program.progress_tracker = @progress_tracker
       program.warnings = @warnings
-      program.warnings_exclude = @warnings_exclude.map { |p| File.expand_path p }
-      program.error_on_warnings = @error_on_warnings
       program
     end
 

src/compiler/crystal/macros/methods.cr

@@ -37,7 +37,7 @@ module Crystal
       filename
     end
 
-    delegate report_warning_at, to: @program
+    delegate warnings, to: @program
 
     def interpret_top_level_call(node)
       interpret_top_level_call?(node) ||
@@ -2199,7 +2199,7 @@ module Crystal
           ArrayLiteral.map(@names) { |name| MacroId.new(name) }
         end
       when "global"
-        interpreter.report_warning_at(name_loc, "Deprecated Path#global. Use `#global?` instead")
+        interpreter.warnings.add_warning_at(name_loc, "Deprecated Path#global. Use `#global?` instead")
         interpret_check_args { BoolLiteral.new(@global) }
       when "global?"
         interpret_check_args { BoolLiteral.new(@global) }

src/compiler/crystal/semantic/abstract_def_checker.cr

@@ -375,7 +375,7 @@ class Crystal::AbstractDefChecker
       impl_param = impl_method.args[i]
       base_param = base_method.args[i]
       unless impl_param.external_name == base_param.external_name
-        @program.report_warning(impl_param, "positional parameter '#{impl_param.external_name}' corresponds to parameter '#{base_param.external_name}' of the overridden method #{Call.def_full_name(base_type, base_method)}, which has a different name and may affect named argument passing")
+        @program.warnings.add_warning(impl_param, "positional parameter '#{impl_param.external_name}' corresponds to parameter '#{base_param.external_name}' of the overridden method #{Call.def_full_name(base_type, base_method)}, which has a different name and may affect named argument passing")
       end
     end
   end

src/compiler/crystal/semantic/ast.cr

@@ -12,15 +12,6 @@ module Crystal
       ::raise exception_type.for_node(self, message, inner)
     end
 
-    def warning(message, inner = nil, exception_type = Crystal::TypeException)
-      # TODO extract message formatting from exceptions
-      String.build do |io|
-        exception = exception_type.for_node(self, message, inner)
-        exception.warning = true
-        exception.append_to_s(io, nil)
-      end
-    end
-
     def simple_literal?
       case self
       when Nop, NilLiteral, BoolLiteral, NumberLiteral, CharLiteral,

src/compiler/crystal/semantic/top_level_visitor.cr

@@ -507,11 +507,11 @@ class Crystal::TopLevelVisitor < Crystal::SemanticVisitor
         link_annotation = LinkAnnotation.from(ann)
 
         if link_annotation.static?
-          @program.report_warning(ann, "specifying static linking for individual libraries is deprecated")
+          @program.warnings.add_warning(ann, "specifying static linking for individual libraries is deprecated")
         end
 
         if ann.args.size > 1
-          @program.report_warning(ann, "using non-named arguments for Link annotations is deprecated")
+          @program.warnings.add_warning(ann, "using non-named arguments for Link annotations is deprecated")
         end
 
         type.add_link_annotation(link_annotation)

src/compiler/crystal/semantic/warnings.cr

@@ -1,68 +1,28 @@
+require "../warnings"
+
 module Crystal
   class Program
-    # Which kind of warnings wants to be detected.
-    property warnings : Warnings = Warnings::All
-
-    # Paths to ignore for warnings detection.
-    property warnings_exclude : Array(String) = [] of String
-
-    # Detected warning failures.
-    property warning_failures = [] of String
-
-    # If `true` compiler will error if warnings are found.
-    property error_on_warnings : Bool = false
+    # Warning settings and all detected warnings.
+    property warnings = WarningCollection.new
 
     @deprecated_constants_detected = Set(String).new
     @deprecated_methods_detected = Set(String).new
     @deprecated_macros_detected = Set(String).new
 
-    def report_warning(node : ASTNode, message : String)
-      return unless self.warnings.all?
-      return if self.ignore_warning_due_to_location?(node.location)
-
-      self.warning_failures << node.warning(message)
-    end
-
-    def report_warning_at(location : Location?, message : String)
-      return unless self.warnings.all?
-      return if self.ignore_warning_due_to_location?(location)
-
-      if location
-        message = String.build do |io|
-          exception = SyntaxException.new message, location.line_number, location.column_number, location.filename
-          exception.warning = true
-          exception.append_to_s(io, nil)
-        end
-      end
-
-      self.warning_failures << message
-    end
-
-    def ignore_warning_due_to_location?(location : Location?)
-      return false unless location
-
-      filename = location.original_filename
-      return false unless filename
-
-      @program.warnings_exclude.any? do |path|
-        filename.starts_with?(path)
-      end
-    end
-
     def check_deprecated_constant(const : Const, node : Path)
-      return unless @warnings.all?
+      return unless @warnings.level.all?
 
       check_deprecation(const, node, @deprecated_constants_detected)
     end
 
     def check_call_to_deprecated_macro(a_macro : Macro, call : Call)
-      return unless @warnings.all?
+      return unless @warnings.level.all?
 
       check_deprecation(a_macro, call, @deprecated_macros_detected)
     end
 
     def check_call_to_deprecated_method(node : Call)
-      return unless @warnings.all?
+      return unless @warnings.level.all?
       return if compiler_expanded_call(node)
 
       node.target_defs.try &.each do |target_def|
@@ -74,7 +34,7 @@ module Crystal
       if (ann = object.annotation(self.deprecated_annotation)) &&
          (deprecated_annotation = DeprecatedAnnotation.from(ann))
         use_location = use_site.location.try(&.macro_location) || use_site.location
-        return if !use_location || ignore_warning_due_to_location?(use_location)
+        return if !use_location || @warnings.ignore_warning_due_to_location?(use_location)
 
         # skip warning if the use site was already informed
         name = object.short_reference
@@ -89,7 +49,7 @@ module Crystal
           end
         end
 
-        @warning_failures << use_site.warning(full_message)
+        @warnings.infos << use_site.warning(full_message)
       end
     end
 
@@ -163,29 +123,4 @@ module Crystal
       to_s
     end
   end
-
-  class Command
-    def report_warnings
-      compiler = @compiler
-      return unless compiler
-
-      program = compiler.program?
-      return unless program
-      return if program.warning_failures.empty?
-
-      program.warning_failures.each do |message|
-        STDERR.puts message
-        STDERR.puts "\n"
-      end
-      STDERR.puts "A total of #{program.warning_failures.size} warnings were found."
-    end
-
-    def warnings_fail_on_exit?
-      compiler = @compiler
-      return false unless compiler
-
-      program = compiler.program
-      program.error_on_warnings && program.warning_failures.size > 0
-    end
-  end
 end