mirror of
				https://gitea.invidious.io/iv-org/shard-ameba.git
				synced 2024-08-15 00:53:29 +00:00 
			
		
		
		
	Add inline directives parsing and disabling
This commit is contained in:
		
							parent
							
								
									55b66e7975
								
							
						
					
					
						commit
						9f85b16e09
					
				
					 9 changed files with 212 additions and 7 deletions
				
			
		
							
								
								
									
										11
									
								
								README.md
									
										
									
									
									
								
							
							
						
						
									
										11
									
								
								README.md
									
										
									
									
									
								
							| 
						 | 
					@ -106,6 +106,17 @@ It allows to configure rule properties, disable specific rules and exclude sourc
 | 
				
			||||||
 | 
					
 | 
				
			||||||
Generate new file by running `ameba --gen-config`.
 | 
					Generate new file by running `ameba --gen-config`.
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					### Inline disabling
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					One or more rules can't be disabled using inline directives:
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					```crystal
 | 
				
			||||||
 | 
					# ameba:disable LargeNumbers
 | 
				
			||||||
 | 
					time = Time.epoch(1483859302)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					time = Time.epoch(1483859302) # ameba:disable LargeNumbers
 | 
				
			||||||
 | 
					```
 | 
				
			||||||
 | 
					
 | 
				
			||||||
## Writing a new Rule
 | 
					## Writing a new Rule
 | 
				
			||||||
 | 
					
 | 
				
			||||||
Adding a new rule is as simple as inheriting from `Ameba::Rule::Base` struct and implementing
 | 
					Adding a new rule is as simple as inheriting from `Ameba::Rule::Base` struct and implementing
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -36,6 +36,27 @@ module Ameba::Formatter
 | 
				
			||||||
        subject.finished [Source.new ""]
 | 
					        subject.finished [Source.new ""]
 | 
				
			||||||
        output.to_s.should contain "Finished in"
 | 
					        output.to_s.should contain "Finished in"
 | 
				
			||||||
      end
 | 
					      end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      context "when errors found" do
 | 
				
			||||||
 | 
					        it "writes each error" do
 | 
				
			||||||
 | 
					          s = Source.new("").tap do |s|
 | 
				
			||||||
 | 
					            s.error(DummyRule.new, 1, 1, "DummyRuleError")
 | 
				
			||||||
 | 
					            s.error(NamedRule.new, 1, 2, "NamedRuleError")
 | 
				
			||||||
 | 
					          end
 | 
				
			||||||
 | 
					          subject.finished [s]
 | 
				
			||||||
 | 
					          log = output.to_s
 | 
				
			||||||
 | 
					          log.should contain "1 inspected, 2 failures."
 | 
				
			||||||
 | 
					          log.should contain "DummyRuleError"
 | 
				
			||||||
 | 
					          log.should contain "NamedRuleError"
 | 
				
			||||||
 | 
					        end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        it "does not write disabled errors" do
 | 
				
			||||||
 | 
					          s = Source.new ""
 | 
				
			||||||
 | 
					          s.error(DummyRule.new, 1, 1, "DummyRuleError", :disabled)
 | 
				
			||||||
 | 
					          subject.finished [s]
 | 
				
			||||||
 | 
					          output.to_s.should contain "1 inspected, 0 failures."
 | 
				
			||||||
 | 
					        end
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
end
 | 
					end
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
							
								
								
									
										87
									
								
								spec/ameba/inline_comments_spec.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										87
									
								
								spec/ameba/inline_comments_spec.cr
									
										
									
									
									
										Normal file
									
								
							| 
						 | 
					@ -0,0 +1,87 @@
 | 
				
			||||||
 | 
					require "../spec_helper"
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					module Ameba
 | 
				
			||||||
 | 
					  describe InlineComments do
 | 
				
			||||||
 | 
					    it "disables a rule with a comment directive" do
 | 
				
			||||||
 | 
					      s = Source.new %Q(
 | 
				
			||||||
 | 
					        # ameba:disable #{NamedRule.name}
 | 
				
			||||||
 | 
					        Time.epoch(1483859302)
 | 
				
			||||||
 | 
					      )
 | 
				
			||||||
 | 
					      s.error(NamedRule.new, 3, 12, "Error!")
 | 
				
			||||||
 | 
					      s.should be_valid
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    it "disables a rule with a line that ends with a comment directive" do
 | 
				
			||||||
 | 
					      s = Source.new %Q(
 | 
				
			||||||
 | 
					        Time.epoch(1483859302) # ameba:disable #{NamedRule.name}
 | 
				
			||||||
 | 
					      )
 | 
				
			||||||
 | 
					      s.error(NamedRule.new, 2, 12, "Error!")
 | 
				
			||||||
 | 
					      s.should be_valid
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    it "does not disable a rule of a different name" do
 | 
				
			||||||
 | 
					      s = Source.new %Q(
 | 
				
			||||||
 | 
					        # ameba:disable WrongName
 | 
				
			||||||
 | 
					        Time.epoch(1483859302)
 | 
				
			||||||
 | 
					      )
 | 
				
			||||||
 | 
					      s.error(NamedRule.new, 3, 12, "Error!")
 | 
				
			||||||
 | 
					      s.should_not be_valid
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    it "disables a rule if multiple rule names provided" do
 | 
				
			||||||
 | 
					      s = Source.new %Q(
 | 
				
			||||||
 | 
					        # ameba:disable SomeRule LargeNumbers #{NamedRule.name} SomeOtherRule
 | 
				
			||||||
 | 
					        Time.epoch(1483859302)
 | 
				
			||||||
 | 
					      )
 | 
				
			||||||
 | 
					      s.error(NamedRule.new, 3, 12, "")
 | 
				
			||||||
 | 
					      s.should be_valid
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    it "disables a rule if multiple rule names are separated by comma" do
 | 
				
			||||||
 | 
					      s = Source.new %Q(
 | 
				
			||||||
 | 
					        # ameba:disable SomeRule, LargeNumbers, #{NamedRule.name}, SomeOtherRule
 | 
				
			||||||
 | 
					        Time.epoch(1483859302)
 | 
				
			||||||
 | 
					      )
 | 
				
			||||||
 | 
					      s.error(NamedRule.new, 3, 12, "")
 | 
				
			||||||
 | 
					      s.should be_valid
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    it "does not disable if multiple rule names used without required one" do
 | 
				
			||||||
 | 
					      s = Source.new %(
 | 
				
			||||||
 | 
					        # ameba:disable SomeRule, SomeOtherRule LargeNumbers
 | 
				
			||||||
 | 
					        Time.epoch(1483859302)
 | 
				
			||||||
 | 
					      )
 | 
				
			||||||
 | 
					      s.error(NamedRule.new, 3, 12, "")
 | 
				
			||||||
 | 
					      s.should_not be_valid
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    it "does not disable if comment directive has wrong place" do
 | 
				
			||||||
 | 
					      s = Source.new %Q(
 | 
				
			||||||
 | 
					        # ameba:disable #{NamedRule.name}
 | 
				
			||||||
 | 
					        #
 | 
				
			||||||
 | 
					        Time.epoch(1483859302)
 | 
				
			||||||
 | 
					      )
 | 
				
			||||||
 | 
					      s.error(NamedRule.new, 4, 12, "")
 | 
				
			||||||
 | 
					      s.should_not be_valid
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    it "does not disable if comment directive added to the wrong line" do
 | 
				
			||||||
 | 
					      s = Source.new %Q(
 | 
				
			||||||
 | 
					        if use_epoch? # ameba:disable #{NamedRule.name}
 | 
				
			||||||
 | 
					          Time.epoch(1483859302)
 | 
				
			||||||
 | 
					        end
 | 
				
			||||||
 | 
					      )
 | 
				
			||||||
 | 
					      s.error(NamedRule.new, 3, 12, "")
 | 
				
			||||||
 | 
					      s.should_not be_valid
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    it "does not disable if that is not a comment directive" do
 | 
				
			||||||
 | 
					      s = Source.new %Q(
 | 
				
			||||||
 | 
					        "ameba:disable #{NamedRule.name}"
 | 
				
			||||||
 | 
					        Time.epoch(1483859302)
 | 
				
			||||||
 | 
					      )
 | 
				
			||||||
 | 
					      s.error(NamedRule.new, 3, 12, "")
 | 
				
			||||||
 | 
					      s.should_not be_valid
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					end
 | 
				
			||||||
| 
						 | 
					@ -11,6 +11,19 @@ module Ameba
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  struct NamedRule < Rule::Base
 | 
				
			||||||
 | 
					    properties do
 | 
				
			||||||
 | 
					      description : String = "A rule with a custom name."
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def test(source)
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def self.name
 | 
				
			||||||
 | 
					      "BreakingRule"
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  struct ErrorRule < Rule::Base
 | 
					  struct ErrorRule < Rule::Base
 | 
				
			||||||
    def test(source)
 | 
					    def test(source)
 | 
				
			||||||
      source.error self, 1, 1, "This rule always adds an error"
 | 
					      source.error self, 1, 1, "This rule always adds an error"
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -25,6 +25,7 @@ module Ameba::Formatter
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      failed_sources.each do |source|
 | 
					      failed_sources.each do |source|
 | 
				
			||||||
        source.errors.each do |error|
 | 
					        source.errors.each do |error|
 | 
				
			||||||
 | 
					          next if error.disabled?
 | 
				
			||||||
          output << "#{error.location}\n".colorize(:cyan)
 | 
					          output << "#{error.location}\n".colorize(:cyan)
 | 
				
			||||||
          output << "#{error.rule.name}: #{error.message}\n\n".colorize(:red)
 | 
					          output << "#{error.rule.name}: #{error.message}\n\n".colorize(:red)
 | 
				
			||||||
        end
 | 
					        end
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -2,6 +2,7 @@ module Ameba::Formatter
 | 
				
			||||||
  class FlycheckFormatter < BaseFormatter
 | 
					  class FlycheckFormatter < BaseFormatter
 | 
				
			||||||
    def source_finished(source : Source)
 | 
					    def source_finished(source : Source)
 | 
				
			||||||
      source.errors.each do |e|
 | 
					      source.errors.each do |e|
 | 
				
			||||||
 | 
					        next if e.disabled?
 | 
				
			||||||
        if loc = e.location
 | 
					        if loc = e.location
 | 
				
			||||||
          output.printf "%s:%d:%d: %s: %s\n",
 | 
					          output.printf "%s:%d:%d: %s: %s\n",
 | 
				
			||||||
            source.path, loc.line_number, loc.column_number, "E",
 | 
					            source.path, loc.line_number, loc.column_number, "E",
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -31,7 +31,7 @@ module Ameba::Formatter
 | 
				
			||||||
    private def rule_errors_map(errors)
 | 
					    private def rule_errors_map(errors)
 | 
				
			||||||
      Hash(Rule::Base, Array(Source::Error)).new.tap do |h|
 | 
					      Hash(Rule::Base, Array(Source::Error)).new.tap do |h|
 | 
				
			||||||
        errors.each do |error|
 | 
					        errors.each do |error|
 | 
				
			||||||
          next if error.rule.is_a? Rule::Syntax
 | 
					          next if error.disabled? || error.rule.is_a? Rule::Syntax
 | 
				
			||||||
          h[error.rule] ||= Array(Source::Error).new
 | 
					          h[error.rule] ||= Array(Source::Error).new
 | 
				
			||||||
          h[error.rule] << error
 | 
					          h[error.rule] << error
 | 
				
			||||||
        end
 | 
					        end
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
							
								
								
									
										63
									
								
								src/ameba/inline_comments.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										63
									
								
								src/ameba/inline_comments.cr
									
										
									
									
									
										Normal file
									
								
							| 
						 | 
					@ -0,0 +1,63 @@
 | 
				
			||||||
 | 
					module Ameba
 | 
				
			||||||
 | 
					  # A module that represents inline comments parsing and processing logic.
 | 
				
			||||||
 | 
					  module InlineComments
 | 
				
			||||||
 | 
					    COMMENT_DIRECTIVE_REGEX = Regex.new "# ameba : (\\w+) ([\\w, ]+)".gsub(" ", "\\s*")
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    # Returns true if current location is disabled for a particular rule,
 | 
				
			||||||
 | 
					    # false otherwise.
 | 
				
			||||||
 | 
					    #
 | 
				
			||||||
 | 
					    # Location is disabled in two cases:
 | 
				
			||||||
 | 
					    #   1. The line of the location ends with a comment directive.
 | 
				
			||||||
 | 
					    #   2. The line above the location is a comment directive.
 | 
				
			||||||
 | 
					    #
 | 
				
			||||||
 | 
					    # For example, here is two examples of disabled location:
 | 
				
			||||||
 | 
					    #
 | 
				
			||||||
 | 
					    # ```
 | 
				
			||||||
 | 
					    # # ameba:disable LargeNumbers
 | 
				
			||||||
 | 
					    # Time.epoch(1483859302)
 | 
				
			||||||
 | 
					    #
 | 
				
			||||||
 | 
					    # Time.epoch(1483859302) # ameba:disable LargeNumbers
 | 
				
			||||||
 | 
					    # ```
 | 
				
			||||||
 | 
					    #
 | 
				
			||||||
 | 
					    # But here are examples which are not considered as disabled location:
 | 
				
			||||||
 | 
					    #
 | 
				
			||||||
 | 
					    # ```
 | 
				
			||||||
 | 
					    # # ameba:disable LargeNumbers
 | 
				
			||||||
 | 
					    # #
 | 
				
			||||||
 | 
					    # Time.epoch(1483859302)
 | 
				
			||||||
 | 
					    #
 | 
				
			||||||
 | 
					    # if use_epoch? # ameba:disable LargeNumbers
 | 
				
			||||||
 | 
					    #   Time.epoch(1483859302)
 | 
				
			||||||
 | 
					    # end
 | 
				
			||||||
 | 
					    # ```
 | 
				
			||||||
 | 
					    #
 | 
				
			||||||
 | 
					    def location_disabled?(location, rule)
 | 
				
			||||||
 | 
					      return false unless line_number = location.try &.line_number.try &.- 1
 | 
				
			||||||
 | 
					      return false unless line = lines[line_number]?
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      line_disabled?(line, rule) ||
 | 
				
			||||||
 | 
					        (line_number > 0 &&
 | 
				
			||||||
 | 
					          (prev_line = lines[line_number - 1]) &&
 | 
				
			||||||
 | 
					          comment?(prev_line) &&
 | 
				
			||||||
 | 
					          line_disabled?(prev_line, rule))
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    private def comment?(line)
 | 
				
			||||||
 | 
					      line.lstrip.starts_with? '#'
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    private def line_disabled?(line, rule)
 | 
				
			||||||
 | 
					      return false unless inline_comment = parse_inline_comment(line)
 | 
				
			||||||
 | 
					      inline_comment[:action] == "disable" && inline_comment[:rules].includes?(rule)
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    private def parse_inline_comment(line)
 | 
				
			||||||
 | 
					      if comment = COMMENT_DIRECTIVE_REGEX.match(line)
 | 
				
			||||||
 | 
					        {
 | 
				
			||||||
 | 
					          action: comment[1],
 | 
				
			||||||
 | 
					          rules:  comment[2].split(/[\s,]/, remove_empty: true),
 | 
				
			||||||
 | 
					        }
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					end
 | 
				
			||||||
| 
						 | 
					@ -2,6 +2,8 @@ module Ameba
 | 
				
			||||||
  # An entity that represents a Crystal source file.
 | 
					  # An entity that represents a Crystal source file.
 | 
				
			||||||
  # Has path, lines of code and errors reported by rules.
 | 
					  # Has path, lines of code and errors reported by rules.
 | 
				
			||||||
  class Source
 | 
					  class Source
 | 
				
			||||||
 | 
					    include InlineComments
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    # Represents an error caught by Ameba.
 | 
					    # Represents an error caught by Ameba.
 | 
				
			||||||
    #
 | 
					    #
 | 
				
			||||||
    # Each error has the rule that created this error,
 | 
					    # Each error has the rule that created this error,
 | 
				
			||||||
| 
						 | 
					@ -9,7 +11,12 @@ module Ameba
 | 
				
			||||||
    record Error,
 | 
					    record Error,
 | 
				
			||||||
      rule : Rule::Base,
 | 
					      rule : Rule::Base,
 | 
				
			||||||
      location : Crystal::Location?,
 | 
					      location : Crystal::Location?,
 | 
				
			||||||
      message : String
 | 
					      message : String,
 | 
				
			||||||
 | 
					      status : Symbol? do
 | 
				
			||||||
 | 
					      def disabled?
 | 
				
			||||||
 | 
					        status == :disabled
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    # Path to the source file.
 | 
					    # Path to the source file.
 | 
				
			||||||
    getter path : String
 | 
					    getter path : String
 | 
				
			||||||
| 
						 | 
					@ -42,8 +49,9 @@ module Ameba
 | 
				
			||||||
    # source.error rule, location, "Line too long"
 | 
					    # source.error rule, location, "Line too long"
 | 
				
			||||||
    # ```
 | 
					    # ```
 | 
				
			||||||
    #
 | 
					    #
 | 
				
			||||||
    def error(rule : Rule::Base, location, message : String)
 | 
					    def error(rule : Rule::Base, location, message : String, status = nil)
 | 
				
			||||||
      errors << Error.new rule, location, message
 | 
					      status ||= :disabled if location_disabled?(location, rule.name)
 | 
				
			||||||
 | 
					      errors << Error.new rule, location, message, status
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    # Adds new error to the list of errors using line and column number.
 | 
					    # Adds new error to the list of errors using line and column number.
 | 
				
			||||||
| 
						 | 
					@ -52,9 +60,9 @@ module Ameba
 | 
				
			||||||
    # source.error rule, line_number, column_number, "Bad code"
 | 
					    # source.error rule, line_number, column_number, "Bad code"
 | 
				
			||||||
    # ```
 | 
					    # ```
 | 
				
			||||||
    #
 | 
					    #
 | 
				
			||||||
    def error(rule : Rule::Base, l : Int32, c : Int32, message : String)
 | 
					    def error(rule : Rule::Base, l, c, message : String, status = nil)
 | 
				
			||||||
      location = Crystal::Location.new path, l, c
 | 
					      location = Crystal::Location.new path, l, c
 | 
				
			||||||
      error rule, location, message
 | 
					      error rule, location, message, status
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    # Indicates whether source is valid or not.
 | 
					    # Indicates whether source is valid or not.
 | 
				
			||||||
| 
						 | 
					@ -68,7 +76,7 @@ module Ameba
 | 
				
			||||||
    # ```
 | 
					    # ```
 | 
				
			||||||
    #
 | 
					    #
 | 
				
			||||||
    def valid?
 | 
					    def valid?
 | 
				
			||||||
      errors.empty?
 | 
					      errors.reject(&.disabled?).empty?
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    # Returns lines of code splitted by new line character.
 | 
					    # Returns lines of code splitted by new line character.
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue