diff --git a/config/default.yml b/config/default.yml index 664df47f..2d555417 100644 --- a/config/default.yml +++ b/config/default.yml @@ -61,6 +61,7 @@ linters: - Style/AlignParameters - Style/BlockNesting - Style/FileName + - Style/FinalNewline - Style/IfUnlessModifier - Style/IndentationWidth - Style/Next diff --git a/lib/haml_lint.rb b/lib/haml_lint.rb index c22f13c1..21a5d2f0 100644 --- a/lib/haml_lint.rb +++ b/lib/haml_lint.rb @@ -2,7 +2,7 @@ require 'haml_lint/exceptions' require 'haml_lint/configuration' require 'haml_lint/configuration_loader' -require 'haml_lint/parser' +require 'haml_lint/document' require 'haml_lint/haml_visitor' require 'haml_lint/lint' require 'haml_lint/linter_registry' @@ -11,6 +11,7 @@ require 'haml_lint/logger' require 'haml_lint/reporter' require 'haml_lint/report' +require 'haml_lint/linter_selector' require 'haml_lint/file_finder' require 'haml_lint/runner' require 'haml_lint/utils' diff --git a/lib/haml_lint/cli.rb b/lib/haml_lint/cli.rb index 2c2a362f..aecce763 100644 --- a/lib/haml_lint/cli.rb +++ b/lib/haml_lint/cli.rb @@ -1,3 +1,4 @@ +require 'haml_lint' require 'haml_lint/options' require 'sysexits' @@ -5,8 +6,8 @@ module HamlLint # Command line application interface. class CLI - attr_accessor :options - + # Create a CLI that outputs to the specified logger. + # # @param logger [HamlLint::Logger] def initialize(logger) @log = logger @@ -16,7 +17,7 @@ def initialize(logger) # based on those arguments. # # @param args [Array] command line arguments - # @return [Fixnum] exit status returned by the application + # @return [Integer] exit status code def run(args) options = HamlLint::Options.new.parse(args) act_on_options(options) @@ -28,6 +29,9 @@ def run(args) attr_reader :log + # Given the provided options, execute the appropriate command. + # + # @return [Integer] exit status code def act_on_options(options) log.color_enabled = options.fetch(:color, log.tty?) @@ -48,6 +52,8 @@ def act_on_options(options) end end + # Outputs a message and returns an appropriate error code for the specified + # exception. def handle_exception(ex) case ex when HamlLint::Exceptions::ConfigurationError @@ -69,21 +75,27 @@ def handle_exception(ex) end end + # Scans the files specified by the given options for lints. + # + # @return [Integer] exit status code def scan_for_lints(options) report = Runner.new.run(options) print_report(report, options) report.failed? ? Sysexits::EX_DATAERR : Sysexits::EX_OK end + # Outputs a report of the linter run using the specified reporter. def print_report(report, options) - reporter = options.fetch(:reporter, Reporter::DefaultReporter).new(log, report) - reporter.report_lints + reporter = options.fetch(:reporter, + HamlLint::Reporter::DefaultReporter).new(log) + reporter.display_report(report) end + # Outputs a list of all currently available linters. def print_available_linters log.info 'Available linters:' - linter_names = LinterRegistry.linters.map do |linter| + linter_names = HamlLint::LinterRegistry.linters.map do |linter| linter.name.split('::').last end @@ -92,10 +104,11 @@ def print_available_linters end end + # Outputs a list of currently available reporters. def print_available_reporters log.info 'Available reporters:' - reporter_names = Reporter.descendants.map do |reporter| + reporter_names = HamlLint::Reporter.descendants.map do |reporter| reporter.name.split('::').last.sub(/Reporter$/, '').downcase end @@ -104,14 +117,18 @@ def print_available_reporters end end + # Outputs help documentation. def print_help(options) log.log options[:help] end + # Outputs the application name and version. def print_version - log.log "#{APP_NAME} #{HamlLint::VERSION}" + log.log "#{HamlLint::APP_NAME} #{HamlLint::VERSION}" end + # Outputs the backtrace of an exception with instructions on how to report + # the issue. def print_unexpected_exception(ex) log.bold_error ex.message log.error ex.backtrace.join("\n") diff --git a/lib/haml_lint/configuration.rb b/lib/haml_lint/configuration.rb index 69e30dea..a488683f 100644 --- a/lib/haml_lint/configuration.rb +++ b/lib/haml_lint/configuration.rb @@ -1,6 +1,12 @@ module HamlLint - # Stores configuration for haml-lint. + # Stores runtime configuration for the application. + # + # The purpose of this class is to validate and ensure all configurations + # satisfy some basic pre-conditions so other parts of the application don't + # have to check the configuration for errors. It should have no knowledge of + # how these configuration values are ultimately used. class Configuration + # Internal hash storing the configuration. attr_reader :hash # Creates a configuration from the given options hash. @@ -11,6 +17,14 @@ def initialize(options) validate end + # Access the configuration as if it were a hash. + # + # @param key [String] + # @return [Array,Hash,Number,String] + def [](key) + @hash[key] + end + # Compares this configuration with another. # # @param other [HamlLint::Configuration] @@ -30,19 +44,9 @@ def for_linter(linter) linter.name.split('::').last when HamlLint::Linter linter.name - else - linter.to_s end - smart_merge(@hash['linters']['ALL'], - @hash['linters'].fetch(linter_name, {})).freeze - end - - # Returns whether the specified linter is enabled by this configuration. - # - # @param linter [HamlLint::Linter,String] - def linter_enabled?(linter) - for_linter(linter)['enabled'] != false + @hash['linters'].fetch(linter_name, {}).dup.freeze end # Merges the given configuration with this one, returning a new @@ -56,18 +60,14 @@ def merge(config) private - # Validates the configuration for any invalid options, normalizing it where - # possible. - def validate - @hash = convert_nils_to_empty_hashes(@hash) - ensure_linter_section_exists(@hash) - end - + # Merge two hashes such that nested hashes are merged rather than replaced. + # + # @param parent [Hash] + # @param child [Hash] + # @return [Hash] def smart_merge(parent, child) parent.merge(child) do |_key, old, new| case old - when Array - old + Array(new) when Hash smart_merge(old, new) else @@ -76,21 +76,32 @@ def smart_merge(parent, child) end end - def ensure_linter_section_exists(hash) - hash['linters'] ||= {} - hash['linters']['ALL'] ||= {} + # Validates the configuration for any invalid options, normalizing it where + # possible. + def validate + ensure_exclude_option_array_exists + ensure_linter_section_exists + ensure_linter_include_exclude_arrays_exist + end + + # Ensures the `exclude` global option is an array. + def ensure_exclude_option_array_exists + @hash['exclude'] = Array(@hash['exclude']) end - def convert_nils_to_empty_hashes(hash) - hash.each_with_object({}) do |(key, value), h| - h[key] = - case value - when nil then {} - when Hash then convert_nils_to_empty_hashes(value) - else - value - end - h + # Ensures the `linters` configuration section exists. + def ensure_linter_section_exists + @hash['linters'] ||= {} + end + + # Ensure `include` and `exclude` options for linters are arrays + # (since users can specify a single string glob pattern for convenience) + def ensure_linter_include_exclude_arrays_exist + @hash['linters'].keys.each do |linter_name| + %w[include exclude].each do |option| + linter_config = @hash['linters'][linter_name] + linter_config[option] = Array(linter_config[option]) + end end end end diff --git a/lib/haml_lint/configuration_loader.rb b/lib/haml_lint/configuration_loader.rb index 3e993848..5e3bde7b 100644 --- a/lib/haml_lint/configuration_loader.rb +++ b/lib/haml_lint/configuration_loader.rb @@ -4,10 +4,12 @@ module HamlLint # Manages configuration file loading. class ConfigurationLoader - DEFAULT_CONFIG_PATH = File.join(HAML_LINT_HOME, 'config', 'default.yml') + DEFAULT_CONFIG_PATH = File.join(HamlLint::HOME, 'config', 'default.yml') CONFIG_FILE_NAME = '.haml-lint.yml' class << self + # Load configuration file given the current working directory the + # application is running within. def load_applicable_config directory = File.expand_path(Dir.pwd) config_file = possible_config_files(directory).find(&:file?) @@ -19,33 +21,42 @@ def load_applicable_config end end + # Loads the built-in default configuration. def default_configuration @default_config ||= load_from_file(DEFAULT_CONFIG_PATH) end # Loads a configuration, ensuring it extends the default configuration. + # + # @param file [String] + # @return [HamlLint::Configuration] def load_file(file) config = load_from_file(file) default_configuration.merge(config) - rescue => error + rescue Psych::SyntaxError, Errno::ENOENT => error raise HamlLint::Exceptions::ConfigurationError, "Unable to load configuration from '#{file}': #{error}", error.backtrace end + # Creates a configuration from the specified hash, ensuring it extends the + # default configuration. + # + # @param hash [Hash] + # @return [HamlLint::Configuration] def load_hash(hash) config = HamlLint::Configuration.new(hash) default_configuration.merge(config) - rescue => error - raise HamlLint::Exceptions::ConfigurationError, - "Unable to load configuration from '#{file}': #{error}", - error.backtrace end private + # Parses and loads a configuration from the given file. + # + # @param file [String] + # @return [HamlLint::Configuration] def load_from_file(file) hash = if yaml = YAML.load_file(file) @@ -57,6 +68,11 @@ def load_from_file(file) HamlLint::Configuration.new(hash) end + # Returns a list of possible configuration files given the context of the + # specified directory. + # + # @param directory [String] + # @return [Array] def possible_config_files(directory) files = Pathname.new(directory) .enum_for(:ascend) diff --git a/lib/haml_lint/constants.rb b/lib/haml_lint/constants.rb index a97e1332..c7228cdb 100644 --- a/lib/haml_lint/constants.rb +++ b/lib/haml_lint/constants.rb @@ -1,6 +1,6 @@ # Global application constants. module HamlLint - HAML_LINT_HOME = File.expand_path(File.join(File.dirname(__FILE__), '..', '..')) + HOME = File.expand_path(File.join(File.dirname(__FILE__), '..', '..')) APP_NAME = 'haml-lint' REPO_URL = 'https://github.com/brigade/haml-lint' diff --git a/lib/haml_lint/document.rb b/lib/haml_lint/document.rb new file mode 100644 index 00000000..29aea6e3 --- /dev/null +++ b/lib/haml_lint/document.rb @@ -0,0 +1,102 @@ +module HamlLint + # Represents a parsed Haml document and its associated metadata. + class Document + # File name given to source code parsed from just a string. + STRING_SOURCE = '(string)' + + # @return [HamlLint::Configuration] Configuration used to parse template + attr_reader :config + + # @return [String] Haml template file path + attr_reader :file + + # @return [HamlLint::Tree::Node] Root of the parse tree + attr_reader :tree + + # @return [String] original source code + attr_reader :source + + # @return [Array] original source code as an array of lines + attr_reader :source_lines + + # Parses the specified Haml code into a {Document}. + # + # @param source [String] Haml code to parse + # @param options [Hash] + # @option options :file [String] file name of document that was parsed + # @raise [Haml::Parser::Error] if there was a problem parsing the document + def initialize(source, options) + @config = options[:config] + @file = options.fetch(:file, STRING_SOURCE) + + process_source(source) + end + + private + + # @param source [String] Haml code to parse + # @raise [Haml::Parser::Error] if there was a problem parsing the document + def process_source(source) + @source = strip_frontmatter(source) + @source_lines = @source.split("\n") + + @tree = process_tree(Haml::Parser.new(@source, Haml::Options.new).parse) + end + + # Processes the {Haml::Parser::ParseNode} tree and returns a tree composed + # of friendlier {HamlLint::Tree::Node}s. + # + # @param original_tree [Haml::Parser::ParseNode] + # @return [Haml::Tree::Node] + def process_tree(original_tree) + # Remove the trailing empty HAML comment that the parser creates to signal + # the end of the HAML document + if Gem::Requirement.new('~> 4.0.0').satisfied_by?(Gem.loaded_specs['haml'].version) + original_tree.children.pop + end + + @node_transformer = HamlLint::NodeTransformer.new(self) + convert_tree(original_tree) + end + + # Converts a HAML parse tree to a tree of {HamlLint::Tree::Node} objects. + # + # This provides a cleaner interface with which the linters can interact with + # the parse tree. + # + # @param haml_node [Haml::Parser::ParseNode] + # @param parent [Haml::Tree::Node] + # @return [Haml::Tree::Node] + def convert_tree(haml_node, parent = nil) + new_node = @node_transformer.transform(haml_node) + new_node.parent = parent + + new_node.children = haml_node.children.map do |child| + convert_tree(child, new_node) + end + + new_node + end + + # Removes YAML frontmatter + def strip_frontmatter(source) + if config['skip_frontmatter'] && + source =~ / + # From the start of the string + \A + # First-capture match --- followed by optional whitespace up + # to a newline then 0 or more chars followed by an optional newline. + # This matches the --- and the contents of the frontmatter + (---\s*\n.*?\n?) + # From the start of the line + ^ + # Second capture match --- or ... followed by optional whitespace + # and newline. This matches the closing --- for the frontmatter. + (---|\.\.\.)\s*$\n?/mx + source = $POSTMATCH + end + + source + end + end +end diff --git a/lib/haml_lint/file_finder.rb b/lib/haml_lint/file_finder.rb index 78607efe..b3ecad12 100644 --- a/lib/haml_lint/file_finder.rb +++ b/lib/haml_lint/file_finder.rb @@ -1,13 +1,15 @@ require 'find' module HamlLint - # Finds HAML files that should be linted given a specified list of paths, glob + # Finds Haml files that should be linted given a specified list of paths, glob # patterns, and configuration. class FileFinder # List of extensions of files to include under a directory when a directory # is specified instead of a file. VALID_EXTENSIONS = %w[.haml] + # Create a file finder using the specified configuration. + # # @param config [HamlLint::Configuration] def initialize(config) @config = config @@ -22,15 +24,18 @@ def initialize(config) def find(patterns, excluded_patterns) extract_files_from(patterns).reject do |file| excluded_patterns.any? do |exclusion_glob| - ::File.fnmatch?(exclusion_glob, file, - ::File::FNM_PATHNAME | # Wildcards don't match path separators - ::File::FNM_DOTMATCH) # `*` wildcard matches dotfiles + HamlLint::Utils.any_glob_matches?(exclusion_glob, file) end end end private + # Extract the list of matching files given the list of glob patterns, file + # paths, and directories. + # + # @param patterns [Array] + # @return [Array] def extract_files_from(patterns) # rubocop:disable MethodLength files = [] @@ -57,9 +62,13 @@ def extract_files_from(patterns) # rubocop:disable MethodLength end end - files.uniq + files.uniq.sort end + # Whether the given file should be treated as a Haml file. + # + # @param file [String] + # @return [Boolean] def haml_file?(file) return false unless ::FileTest.file?(file) diff --git a/lib/haml_lint/lint.rb b/lib/haml_lint/lint.rb index ed0e4b55..53c067b4 100644 --- a/lib/haml_lint/lint.rb +++ b/lib/haml_lint/lint.rb @@ -1,7 +1,20 @@ module HamlLint # Contains information about a problem or issue with a HAML document. class Lint - attr_reader :filename, :line, :linter, :message, :severity + # @return [String] file path to which the lint applies + attr_reader :filename + + # @return [String] line number of the file the lint corresponds to + attr_reader :line + + # @return [SlimLint::Linter] linter that reported the lint + attr_reader :linter + + # @return [String] error/warning message to display to user + attr_reader :message + + # @return [Symbol] whether this lint is a warning or an error + attr_reader :severity # Creates a new lint. # @@ -18,6 +31,9 @@ def initialize(linter, filename, line, message, severity = :warning) @severity = severity end + # Return whether this lint has a severity of error. + # + # @return [Boolean] def error? @severity == :error end diff --git a/lib/haml_lint/linter.rb b/lib/haml_lint/linter.rb index 1e380c92..5304bf6a 100644 --- a/lib/haml_lint/linter.rb +++ b/lib/haml_lint/linter.rb @@ -1,33 +1,51 @@ module HamlLint # Base implementation for all lint checks. + # + # @abstract class Linter include HamlVisitor - attr_reader :parser, :lints + # List of lints reported by this linter. + # + # @todo Remove once spec/support/shared_linter_context returns an array of + # lints for the subject instead of the linter itself. + attr_reader :lints + # Initializes a linter with the specified configuration. + # # @param config [Hash] configuration for this linter def initialize(config) @config = config @lints = [] - @ruby_parser = nil end - def run(parser) - @parser = parser - visit(parser.tree) + # Runs the linter against the given Haml document. + # + # @param document [HamlLint::Document] + def run(document) + @document = document + @lints = [] + visit(document.tree) + @lints end # Returns the simple name for this linter. + # + # @return [String] def name self.class.name.split('::').last end private - attr_reader :config + attr_reader :config, :document + # Record a lint for reporting back to the user. + # + # @param node [#line] node to extract the line number from + # @param message [String] error/warning to display to the user def add_lint(node, message) - @lints << Lint.new(self, parser.filename, node.line, message) + @lints << HamlLint::Lint.new(self, @document.file, node.line, message) end # Parse Ruby code into an abstract syntax tree. @@ -136,7 +154,7 @@ def next_node(node) def following_node_line(node) [ [node.children.first, next_node(node)].compact.map(&:line), - parser.lines.count + 1, + @document.source_lines.count + 1, ].flatten.min end @@ -148,7 +166,8 @@ def following_node_line(node) def tag_with_inline_text(tag_node) # Normalize each of the lines to ignore the multiline bar (|) and # excess whitespace - parser.lines[(tag_node.line - 1)...(following_node_line(tag_node) - 1)].map do |line| + @document.source_lines[(tag_node.line - 1)...(following_node_line(tag_node) - 1)] + .map do |line| line.strip.gsub(/\|\z/, '').rstrip end.join(' ') end diff --git a/lib/haml_lint/linter/consecutive_comments.rb b/lib/haml_lint/linter/consecutive_comments.rb index 7efc4f2f..55703ea4 100644 --- a/lib/haml_lint/linter/consecutive_comments.rb +++ b/lib/haml_lint/linter/consecutive_comments.rb @@ -3,13 +3,11 @@ module HamlLint class Linter::ConsecutiveComments < Linter include LinterRegistry - MIN_CONSECUTIVE = 2 COMMENT_DETECTOR = ->(child) { child.type == :haml_comment } def visit_root(node) - HamlLint::Utils.find_consecutive( + HamlLint::Utils.for_consecutive_items( node.children, - MIN_CONSECUTIVE, COMMENT_DETECTOR, ) do |group| add_lint(group.first, diff --git a/lib/haml_lint/linter/consecutive_silent_scripts.rb b/lib/haml_lint/linter/consecutive_silent_scripts.rb index 0203d079..3d60c575 100644 --- a/lib/haml_lint/linter/consecutive_silent_scripts.rb +++ b/lib/haml_lint/linter/consecutive_silent_scripts.rb @@ -9,10 +9,10 @@ class Linter::ConsecutiveSilentScripts < Linter end def visit_root(node) - HamlLint::Utils.find_consecutive( + HamlLint::Utils.for_consecutive_items( node.children, - config['max_consecutive'] + 1, SILENT_SCRIPT_DETECTOR, + config['max_consecutive'] + 1, ) do |group| add_lint(group.first, "#{group.count} consecutive Ruby scripts can be merged into " \ diff --git a/lib/haml_lint/linter/line_length.rb b/lib/haml_lint/linter/line_length.rb index 906c152f..5c4a4f88 100644 --- a/lib/haml_lint/linter/line_length.rb +++ b/lib/haml_lint/linter/line_length.rb @@ -9,7 +9,7 @@ def visit_root(_node) max_length = config['max'] dummy_node = Struct.new(:line) - parser.lines.each_with_index do |line, index| + document.source_lines.each_with_index do |line, index| next if line.length <= max_length add_lint(dummy_node.new(index + 1), format(MSG, line.length, max_length)) diff --git a/lib/haml_lint/linter/multiline_pipe.rb b/lib/haml_lint/linter/multiline_pipe.rb index 06d0591b..6302b49f 100644 --- a/lib/haml_lint/linter/multiline_pipe.rb +++ b/lib/haml_lint/linter/multiline_pipe.rb @@ -32,7 +32,7 @@ def visit_plain(node) MULTILINE_PIPE_REGEX = /\s+\|\s*$/ def line_text_for_node(node) - parser.lines[node.line - 1] + document.source_lines[node.line - 1] end def check(node) diff --git a/lib/haml_lint/linter/rubocop.rb b/lib/haml_lint/linter/rubocop.rb index 8d54b852..12f3a5e6 100644 --- a/lib/haml_lint/linter/rubocop.rb +++ b/lib/haml_lint/linter/rubocop.rb @@ -1,4 +1,4 @@ -require 'haml_lint/script_extractor' +require 'haml_lint/ruby_extractor' require 'rubocop' require 'tempfile' @@ -7,33 +7,35 @@ module HamlLint class Linter::RuboCop < Linter include LinterRegistry - def initialize(config) - super - @rubocop = ::RuboCop::CLI.new - @ignored_cops = Array(config['ignored_cops']).flatten - end + def visit_root(_node) + extractor = HamlLint::RubyExtractor.new + extracted_source = extractor.extract(document) - def run(parser) - @parser = parser - @extractor = ScriptExtractor.new(parser) - extracted_code = @extractor.extract.strip + return if extracted_source.source.empty? - # Ensure a final newline in the code we feed to RuboCop - find_lints(extracted_code + "\n") unless extracted_code.empty? + find_lints(extracted_source.source, extracted_source.source_map) end private - def find_lints(code) - original_filename = @parser.filename || 'ruby_script' + # Executes RuboCop against the given Ruby code and records the offenses as + # lints. + # + # @param ruby [String] Ruby code + # @param source_map [Hash] map of Ruby code line numbers to original line + # numbers in the template + def find_lints(ruby, source_map) + rubocop = ::RuboCop::CLI.new + + original_filename = document.file || 'ruby_script' filename = "#{File.basename(original_filename)}.haml_lint.tmp" directory = File.dirname(original_filename) Tempfile.open(filename, directory) do |f| begin - f.write(code) + f.write(ruby) f.close - extract_lints_from_offences(lint_file(f.path)) + extract_lints_from_offenses(lint_file(rubocop, f.path), source_map) ensure f.unlink end @@ -41,36 +43,61 @@ def find_lints(code) end # Defined so we can stub the results in tests - def lint_file(file) - @rubocop.run(%w[--format HamlLint::OffenceCollector] << file) - OffenceCollector.offences + # + # @param rubocop [RuboCop::CLI] + # @param file [String] + # @return [Array] + def lint_file(rubocop, file) + rubocop.run(rubocop_flags << file) + OffenseCollector.offenses end - def extract_lints_from_offences(offences) - offences.select { |offence| !@ignored_cops.include?(offence.cop_name) } - .each do |offence| + # Aggregates RuboCop offenses and converts them to {HamlLint::Lint}s + # suitable for reporting. + # + # @param offenses [Array] + # @param source_map [Hash] + def extract_lints_from_offenses(offenses, source_map) + offenses.select { |offense| !config['ignored_cops'].include?(offense.cop_name) } + .each do |offense| @lints << Lint.new(self, - @parser.filename, - @extractor.source_map[offence.line], - "#{offence.cop_name}: #{offence.message}") + document.file, + source_map[offense.line], + offense.message) end end - end - # Collects offences detected by RuboCop. - class OffenceCollector < ::RuboCop::Formatter::BaseFormatter - attr_accessor :offences + # Returns flags that will be passed to RuboCop CLI. + # + # @return [Array] + def rubocop_flags + flags = %w[--format HamlLint::OffenseCollector] + flags += ['--config', ENV['HAML_LINT_RUBOCOP_CONF']] if ENV['HAML_LINT_RUBOCOP_CONF'] + flags + end + end + # Collects offenses detected by RuboCop. + class OffenseCollector < ::RuboCop::Formatter::BaseFormatter class << self - attr_accessor :offences + # List of offenses reported by RuboCop. + attr_accessor :offenses end + # Executed when RuboCop begins linting. + # + # @param _target_files [Array] def started(_target_files) - self.class.offences = [] + self.class.offenses = [] end - def file_finished(_file, offences) - self.class.offences += offences + # Executed when a file has been scanned by RuboCop, adding the reported + # offenses to our collection. + # + # @param _file [String] + # @param offenses [Array] + def file_finished(_file, offenses) + self.class.offenses += offenses end end end diff --git a/lib/haml_lint/linter/space_before_script.rb b/lib/haml_lint/linter/space_before_script.rb index 91eb0546..a490aff6 100644 --- a/lib/haml_lint/linter/space_before_script.rb +++ b/lib/haml_lint/linter/space_before_script.rb @@ -34,7 +34,7 @@ def visit_tag(node) # rubocop:disable Metrics/CyclomaticComplexity def visit_script(node) # Plain text nodes with interpolation are converted to script nodes, so we # need to ignore them here. - return unless parser.lines[node.line - 1].lstrip.start_with?('=') + return unless document.source_lines[node.line - 1].lstrip.start_with?('=') add_lint(node, MESSAGE_FORMAT % '=') if missing_space?(node) end diff --git a/lib/haml_lint/linter/trailing_whitespace.rb b/lib/haml_lint/linter/trailing_whitespace.rb index b82bbd0a..662da5fe 100644 --- a/lib/haml_lint/linter/trailing_whitespace.rb +++ b/lib/haml_lint/linter/trailing_whitespace.rb @@ -6,7 +6,7 @@ class Linter::TrailingWhitespace < Linter def visit_root(_node) dummy_node = Struct.new(:line) - parser.lines.each_with_index do |line, index| + document.source_lines.each_with_index do |line, index| next unless line =~ /\s+$/ add_lint dummy_node.new(index + 1), 'Line contains trailing whitespace' diff --git a/lib/haml_lint/linter_registry.rb b/lib/haml_lint/linter_registry.rb index 85df4d68..c37d86ea 100644 --- a/lib/haml_lint/linter_registry.rb +++ b/lib/haml_lint/linter_registry.rb @@ -6,12 +6,23 @@ module LinterRegistry @linters = [] class << self + # List of all registered linters. attr_reader :linters - def included(base) - @linters << base + # Executed when a linter includes the {LinterRegistry} module. + # + # This results in the linter being registered with the registry. + # + # @param subclass [Class] + def included(subclass) + @linters << subclass end + # Return a list of {HamlLint::Linter} {Class}es corresponding to the + # specified list of names. + # + # @param linter_names [Array] + # @return [Array] def extract_linters_from(linter_names) linter_names.map do |linter_name| begin diff --git a/lib/haml_lint/linter_selector.rb b/lib/haml_lint/linter_selector.rb new file mode 100644 index 00000000..321eb171 --- /dev/null +++ b/lib/haml_lint/linter_selector.rb @@ -0,0 +1,77 @@ +module HamlLint + # Chooses the appropriate linters to run given the specified configuration. + class LinterSelector + # Creates a selector using the given configuration and additional options. + # + # @param config [HamlLint::Configuration] + # @param options [Hash] + def initialize(config, options) + @config = config + @options = options + end + + # Returns the set of linters to run against the given file. + # + # @param file [String] + # @raise [HamlLint::Exceptions::NoLintersError] when no linters are enabled + # @return [Array] + def linters_for_file(file) + @linters ||= extract_enabled_linters(@config, @options) + @linters.select { |linter| run_linter_on_file?(@config, linter, file) } + end + + private + + # Returns a list of linters that are enabled given the specified + # configuration and additional options. + # + # @param config [HamlLint::Configuration] + # @param options [Hash] + # @return [Array] + def extract_enabled_linters(config, options) + included_linters = LinterRegistry + .extract_linters_from(options.fetch(:included_linters, [])) + + included_linters = LinterRegistry.linters if included_linters.empty? + + excluded_linters = LinterRegistry + .extract_linters_from(options.fetch(:excluded_linters, [])) + + # After filtering out explicitly included/excluded linters, only include + # linters which are enabled in the configuration + linters = (included_linters - excluded_linters).map do |linter_class| + linter_config = config.for_linter(linter_class) + linter_class.new(linter_config) if linter_config['enabled'] + end.compact + + # Highlight condition where all linters were filtered out, as this was + # likely a mistake on the user's part + if linters.empty? + raise HamlLint::Exceptions::NoLintersError, 'No linters specified' + end + + linters + end + + # Whether to run the given linter against the specified file. + # + # @param config [HamlLint::Configuration] + # @param linter [HamlLint::Linter] + # @param file [String] + # @return [Boolean] + def run_linter_on_file?(config, linter, file) + linter_config = config.for_linter(linter) + + if linter_config['include'].any? && + !HamlLint::Utils.any_glob_matches?(linter_config['include'], file) + return false + end + + if HamlLint::Utils.any_glob_matches?(linter_config['exclude'], file) + return false + end + + true + end + end +end diff --git a/lib/haml_lint/node_transformer.rb b/lib/haml_lint/node_transformer.rb index ed96c3ba..d791ad7b 100644 --- a/lib/haml_lint/node_transformer.rb +++ b/lib/haml_lint/node_transformer.rb @@ -7,11 +7,11 @@ module HamlLint # would expect. This class is intended to isolate and handle these cases so # that linters don't have to deal with them. class NodeTransformer - # Creates a node transformer for the given parser context. + # Creates a node transformer for the given Haml document. # - # @param parser [HamlLint::Parser] - def initialize(parser) - @parser = parser + # @param document [HamlLint::Document] + def initialize(document) + @document = document end # Transforms the given {Haml::Parser::ParseNode} into its corresponding @@ -19,7 +19,7 @@ def initialize(parser) def transform(haml_node) node_class = "#{HamlLint::Utils.camel_case(haml_node.type.to_s)}Node" - HamlLint::Tree.const_get(node_class).new(@parser, haml_node) + HamlLint::Tree.const_get(node_class).new(@document, haml_node) rescue NameError # TODO: Wrap in parser error? raise diff --git a/lib/haml_lint/options.rb b/lib/haml_lint/options.rb index 59b4e67e..c9c29315 100644 --- a/lib/haml_lint/options.rb +++ b/lib/haml_lint/options.rb @@ -31,11 +31,6 @@ def parse(args) private def add_linter_options(parser) - parser.on('-e', '--exclude file,...', Array, - 'List of file names to exclude') do |files| - @options[:excluded_files] = files - end - parser.on('-i', '--include-linter linter,...', Array, 'Specify which linters you want to run') do |linters| @options[:included_linters] = linters @@ -48,10 +43,22 @@ def add_linter_options(parser) parser.on('-r', '--reporter reporter', String, 'Specify which reporter you want to use to generate the output') do |reporter| - @options[:reporter] = HamlLint::Reporter.const_get("#{reporter.capitalize}Reporter") + @options[:reporter] = load_reporter_class(reporter.capitalize) end end + # Returns the class of the specified Reporter. + # + # @param reporter_name [String] + # @raise [HamlLint::Exceptions::InvalidCLIOption] if reporter doesn't exist + # @return [Class] + def load_reporter_class(reporter_name) + HamlLint::Reporter.const_get("#{reporter_name}Reporter") + rescue NameError + raise HamlLint::Exceptions::InvalidCLIOption, + "#{reporter_name}Reporter does not exist" + end + def add_file_options(parser) parser.on('-c', '--config config-file', String, 'Specify which configuration file you want to use') do |conf_file| diff --git a/lib/haml_lint/parser.rb b/lib/haml_lint/parser.rb deleted file mode 100644 index fe372709..00000000 --- a/lib/haml_lint/parser.rb +++ /dev/null @@ -1,87 +0,0 @@ -require 'haml' - -module HamlLint - # Parses a HAML document for inspection by linters. - class Parser - attr_reader :contents, :filename, :lines, :tree - - # Creates a parser containing the parse tree of a HAML document. - # - # @param haml_or_filename [String] - # @param options [Hash] - # @option options [true,false] 'skip_frontmatter' Whether to skip - # frontmatter included by frameworks such as Middleman or Jekyll - def initialize(haml_or_filename, options = {}) - if File.exist?(haml_or_filename) - build_from_file(haml_or_filename) - else - build_from_string(haml_or_filename) - end - - process_options(options) - - build_parse_tree - end - - private - - # @param path [String] - def build_from_file(path) - @filename = path - @contents = File.read(path) - end - - # @param haml [String] - def build_from_string(haml) - @contents = haml - end - - def build_parse_tree - original_tree = Haml::Parser.new(@contents, Haml::Options.new).parse - - # Remove the trailing empty HAML comment that the parser creates to signal - # the end of the HAML document - if Gem::Requirement.new('~> 4.0.0').satisfied_by?(Gem.loaded_specs['haml'].version) - original_tree.children.pop - end - - @node_transformer = HamlLint::NodeTransformer.new(self) - @tree = convert_tree(original_tree) - end - - def process_options(options) - if options['skip_frontmatter'] && - @contents =~ / - # From the start of the string - \A - # First-capture match --- followed by optional whitespace up - # to a newline then 0 or more chars followed by an optional newline. - # This matches the --- and the contents of the frontmatter - (---\s*\n.*?\n?) - # From the start of the line - ^ - # Second capture match --- or ... followed by optional whitespace - # and newline. This matches the closing --- for the frontmatter. - (---|\.\.\.)\s*$\n?/mx - @contents = $POSTMATCH - end - - @lines = @contents.split("\n") - end - - # Converts a HAML parse tree to a tree of {HamlLint::Tree::Node} objects. - # - # This provides a cleaner interface with which the linters can interact with - # the parse tree. - def convert_tree(haml_node, parent = nil) - new_node = @node_transformer.transform(haml_node) - new_node.parent = parent - - new_node.children = haml_node.children.map do |child| - convert_tree(child, new_node) - end - - new_node - end - end -end diff --git a/lib/haml_lint/rake_task.rb b/lib/haml_lint/rake_task.rb index f59f1953..185a01cb 100644 --- a/lib/haml_lint/rake_task.rb +++ b/lib/haml_lint/rake_task.rb @@ -1,5 +1,6 @@ require 'rake' require 'rake/tasklib' +require 'haml_lint/constants' module HamlLint # Rake task interface for haml-lint command line interface. @@ -51,6 +52,8 @@ class RakeTask < Rake::TaskLib attr_accessor :quiet # Create the task so it exists in the current namespace. + # + # @param name [Symbol] task name def initialize(name = :haml_lint) @name = name @files = ['.'] # Search for everything under current directory by default @@ -63,27 +66,34 @@ def initialize(name = :haml_lint) private + # Defines the Rake task. def define desc default_description unless ::Rake.application.last_description task(name, [:files]) do |_task, task_args| # Lazy-load so task doesn't affect Rakefile load time - require 'haml_lint' require 'haml_lint/cli' run_cli(task_args) end end + # Executes the CLI given the specified task arguments. + # + # @param task_args [Rake::TaskArguments] def run_cli(task_args) cli_args = ['--config', config] if config logger = quiet ? HamlLint::Logger.silent : HamlLint::Logger.new(STDOUT) result = HamlLint::CLI.new(logger).run(Array(cli_args) + files_to_lint(task_args)) - fail "haml-lint failed with exit code #{result}" unless result == 0 + fail "#{HamlLint::APP_NAME} failed with exit code #{result}" unless result == 0 end + # Returns the list of files that should be linted given the specified task + # arguments. + # + # @param task_args [Rake::TaskArguments] def files_to_lint(task_args) # Note: we're abusing Rake's argument handling a bit here. We call the # first argument `files` but it's actually only the first file--we pull @@ -96,8 +106,13 @@ def files_to_lint(task_args) end # Friendly description that shows the full command that will be executed. + # + # This allows us to change the information displayed by `rake --tasks` based + # on the options passed to the constructor which defined the task. + # + # @return [String] def default_description - description = 'Run `haml-lint' + description = "Run `#{HamlLint::APP_NAME}" description += " --config #{config}" if config description += " #{files.join(' ')}" if files.any? description += ' [files...]`' diff --git a/lib/haml_lint/report.rb b/lib/haml_lint/report.rb index e16050ec..1b4f278b 100644 --- a/lib/haml_lint/report.rb +++ b/lib/haml_lint/report.rb @@ -1,9 +1,16 @@ module HamlLint # Contains information about all lints detected during a scan. class Report + # List of lints that were found. attr_accessor :lints + + # List of files that were linted. attr_reader :files + # Creates a report. + # + # @param lints [Array] lints that were found + # @param files [Array] files that were linted def initialize(lints, files) @lints = lints.sort_by { |l| [l.filename, l.line] } @files = files diff --git a/lib/haml_lint/reporter.rb b/lib/haml_lint/reporter.rb index 7af7bc2b..9e237925 100644 --- a/lib/haml_lint/reporter.rb +++ b/lib/haml_lint/reporter.rb @@ -1,36 +1,42 @@ module HamlLint - # Abstract lint reporter. Subclass and override {#report_lints} to + # Abstract lint reporter. Subclass and override {#display_report} to # implement a custom lint reporter. # # @abstract class Reporter - attr_reader :lints - attr_reader :files - + # Creates the reporter that will display the given report. + # # @param logger [HamlLint::Logger] - # @param report [HamlLint::Report] - def initialize(logger, report) + def initialize(logger) @log = logger - @lints = report.lints - @files = report.files end # Implemented by subclasses to display lints from a {HamlLint::Report}. - def report_lints - raise NotImplementedError + # + # @param report [HamlLint::Report] + def display_report(report) + raise NotImplementedError, + "Implement `display_report` to display #{report}" end - # Keep tracking all the descendants of this class for the list of available reporters + # Keep tracking all the descendants of this class for the list of available + # reporters. + # + # @return [Array] def self.descendants @descendants ||= [] end + # Executed when this class is subclassed. + # + # @param descendant [Class] def self.inherited(descendant) descendants << descendant end private + # @return [HamlLint::Logger] logger to send output to attr_reader :log end end diff --git a/lib/haml_lint/reporter/default_reporter.rb b/lib/haml_lint/reporter/default_reporter.rb index e6782161..3296b8ce 100644 --- a/lib/haml_lint/reporter/default_reporter.rb +++ b/lib/haml_lint/reporter/default_reporter.rb @@ -2,8 +2,8 @@ module HamlLint # Outputs lints in a simple format with the filename, line number, and lint # message. class Reporter::DefaultReporter < Reporter - def report_lints - sorted_lints = lints.sort_by { |l| [l.filename, l.line] } + def display_report(report) + sorted_lints = report.lints.sort_by { |l| [l.filename, l.line] } sorted_lints.each do |lint| print_location(lint) diff --git a/lib/haml_lint/reporter/json_reporter.rb b/lib/haml_lint/reporter/json_reporter.rb index f1c22f43..cc07f1ae 100644 --- a/lib/haml_lint/reporter/json_reporter.rb +++ b/lib/haml_lint/reporter/json_reporter.rb @@ -1,29 +1,34 @@ module HamlLint # Outputs report as a JSON document. class Reporter::JsonReporter < Reporter - def report_lints + def display_report(report) + lints = report.lints grouped = lints.group_by(&:filename) - report = { - metadata: { - hamllint_version: VERSION, - ruby_engine: RUBY_ENGINE, - ruby_patchlevel: RUBY_PATCHLEVEL.to_s, - ruby_platform: RUBY_PLATFORM, - }, + report_hash = { + metadata: metadata, files: grouped.map { |l| map_file(l) }, summary: { offense_count: lints.length, target_file_count: grouped.length, - inspected_file_count: files.length, + inspected_file_count: report.files.length, }, } - log.log report.to_json + log.log report_hash.to_json end private + def metadata + { + haml_lint_version: HamlLint::VERSION, + ruby_engine: RUBY_ENGINE, + ruby_patchlevel: RUBY_PATCHLEVEL.to_s, + ruby_platform: RUBY_PLATFORM, + } + end + def map_file(file) { path: file.first, diff --git a/lib/haml_lint/script_extractor.rb b/lib/haml_lint/ruby_extractor.rb similarity index 84% rename from lib/haml_lint/script_extractor.rb rename to lib/haml_lint/ruby_extractor.rb index 9e423dd4..ecaf815e 100644 --- a/lib/haml_lint/script_extractor.rb +++ b/lib/haml_lint/ruby_extractor.rb @@ -19,24 +19,33 @@ module HamlLint # link_to 'Sign In', sign_in_path # end # - class ScriptExtractor + # The translation won't be perfect, and won't make any real sense, but the + # relationship between variable declarations/uses and the flow control graph + # will remain intact. + class RubyExtractor include HamlVisitor - attr_reader :source, :source_map - - def initialize(parser) - @parser = parser - end - - def extract - visit(@parser.tree) - @source = @code.join("\n") + # Stores the extracted source and a map of lines of generated source to the + # original source that created them. + # + # @attr_reader source [String] generated source code + # @attr_reader source_map [Hash] map of line numbers from generated source + # to original source line number + RubySource = Struct.new(:source, :source_map) + + # Extracts Ruby code from Sexp representing a Slim document. + # + # @param document [HamlLint::Document] + # @return [HamlLint::RubyExtractor::RubySource] + def extract(document) + visit(document.tree) + RubySource.new(@source_lines.join("\n"), @source_map) end def visit_root(_node) - @code = [] - @total_lines = 0 + @source_lines = [] @source_map = {} + @line_count = 0 @indent_level = 0 yield # Collect lines of code from children @@ -140,7 +149,7 @@ def add_line(code, node_or_line) indent = (' ' * 2 * indent_level) - @code << indent + code + @source_lines << indent + code original_line = node_or_line.respond_to?(:line) ? node_or_line.line : node_or_line @@ -149,8 +158,8 @@ def add_line(code, node_or_line) # resulting code will span multiple lines, so we need to create a # mapping for each line. (code.count("\n") + 1).times do - @total_lines += 1 - @source_map[@total_lines] = original_line + @line_count += 1 + @source_map[@line_count] = original_line end end diff --git a/lib/haml_lint/runner.rb b/lib/haml_lint/runner.rb index 805b1465..6f211477 100644 --- a/lib/haml_lint/runner.rb +++ b/lib/haml_lint/runner.rb @@ -1,76 +1,76 @@ module HamlLint # Responsible for running the applicable linters against the desired files. class Runner - # Make the list of applicable files available - attr_reader :files - # Runs the appropriate linters against the desired files given the specified # options. # - # @param options [Hash] - # @raise [HamlLint::Exceptions::NoLintersError] when no linters are enabled + # @param [Hash] options + # @option options :config_file [String] path of configuration file to load + # @option options :config [HamlLint::Configuration] configuration to use + # @option options :excluded_files [Array] + # @option options :included_linters [Array] + # @option options :excluded_linters [Array] # @return [HamlLint::Report] a summary of all lints found def run(options = {}) config = load_applicable_config(options) - files = extract_applicable_files(options, config) - linters = extract_enabled_linters(config, options) + files = extract_applicable_files(config, options) - raise HamlLint::Exceptions::NoLintersError, 'No linters specified' if linters.empty? + linter_selector = HamlLint::LinterSelector.new(config, options) - @lints = [] - files.each do |file| - find_lints(file, linters, config) - end - - linters.each do |linter| - @lints += linter.lints - end + lints = files.map do |file| + collect_lints(file, linter_selector, config) + end.flatten - HamlLint::Report.new(@lints, files) + HamlLint::Report.new(lints, files) end private + # Returns the {HamlLint::Configuration} that should be used given the + # specified options. + # + # @param options [Hash] + # @return [HamlLint::Configuration] def load_applicable_config(options) if options[:config_file] HamlLint::ConfigurationLoader.load_file(options[:config_file]) + elsif options[:config] + options[:config] else HamlLint::ConfigurationLoader.load_applicable_config end end - def extract_enabled_linters(config, options) - included_linters = LinterRegistry - .extract_linters_from(options.fetch(:included_linters, [])) - - included_linters = LinterRegistry.linters if included_linters.empty? - - excluded_linters = LinterRegistry - .extract_linters_from(options.fetch(:excluded_linters, [])) - - # After filtering out explicitly included/excluded linters, only include - # linters which are enabled in the configuration - (included_linters - excluded_linters).map do |linter_class| - linter_config = config.for_linter(linter_class) - linter_class.new(linter_config) if linter_config['enabled'] - end.compact - end - - def find_lints(file, linters, config) - parser = Parser.new(file, config.hash) - - linters.each do |linter| - linter.run(parser) + # Runs all provided linters using the specified config against the given + # file. + # + # @param file [String] path to file to lint + # @param linter_selector [HamlLint::LinterSelector] + # @param config [HamlLint::Configuration] + def collect_lints(file, linter_selector, config) + begin + document = HamlLint::Document.new(File.read(file), file: file, config: config) + rescue Haml::Error => ex + return [HamlLint::Lint.new(nil, file, ex.line, ex.to_s, :error)] end - rescue Haml::Error => ex - @lints << Lint.new(nil, file, ex.line, ex.to_s, :error) + + linter_selector.linters_for_file(file).map do |linter| + linter.run(document) + end.flatten end - def extract_applicable_files(options, config) + # Returns the list of files that should be linted given the specified + # configuration and options. + # + # @param config [HamlLint::Configuration] + # @param options [Hash] + # @return [Array] + def extract_applicable_files(config, options) included_patterns = options[:files] - excluded_files = options.fetch(:excluded_files, []) + excluded_patterns = config['exclude'] + excluded_patterns += options.fetch(:excluded_files, []) - HamlLint::FileFinder.new(config).find(included_patterns, excluded_files) + HamlLint::FileFinder.new(config).find(included_patterns, excluded_patterns) end end end diff --git a/lib/haml_lint/tree/node.rb b/lib/haml_lint/tree/node.rb index d776ad4a..b45342c0 100644 --- a/lib/haml_lint/tree/node.rb +++ b/lib/haml_lint/tree/node.rb @@ -14,12 +14,11 @@ class Node # Creates a node wrapping the given {Haml::Parser::ParseNode} struct. # - # @param parser [HamlLint::Parser] parser that created this node + # @param document [HamlLint::Document] Haml document that created this node # @param parse_node [Haml::Parser::ParseNode] parse node created by HAML's parser - def initialize(parser, parse_node) - # TODO: Change signature to take source code object, not parser + def initialize(document, parse_node) @line = parse_node.line - @parser = parser + @document = document @value = parse_node.value @type = parse_node.type end @@ -50,12 +49,12 @@ def source_code if next_node next_node.line - 1 else - @parser.lines.count + 1 + @document.source_lines.count + 1 end - @parser.lines[@line - 1...next_node_line] - .join("\n") - .gsub(/^\s*\z/m, '') # Remove blank lines at the end + @document.source_lines[@line - 1...next_node_line] + .join("\n") + .gsub(/^\s*\z/m, '') # Remove blank lines at the end end def inspect diff --git a/lib/haml_lint/utils.rb b/lib/haml_lint/utils.rb index 4e650931..445ebb32 100644 --- a/lib/haml_lint/utils.rb +++ b/lib/haml_lint/utils.rb @@ -3,6 +3,23 @@ module HamlLint module Utils module_function + # Returns whether a glob pattern (or any of a list of patterns) matches the + # specified file. + # + # This is defined here so our file globbing options are consistent + # everywhere we perform globbing. + # + # @param glob [String, Array] + # @param file [String] + # @return [Boolean] + def any_glob_matches?(globs_or_glob, file) + Array(globs_or_glob).any? do |glob| + ::File.fnmatch?(glob, file, + ::File::FNM_PATHNAME | # Wildcards don't match path separators + ::File::FNM_DOTMATCH) # `*` wildcard matches dotfiles + end + end + # Yields interpolated values within a block of filter text. def extract_interpolated_values(filter_text) Haml::Util.handle_interpolation(filter_text.dump) do |scan| @@ -21,38 +38,61 @@ def camel_case(str) str.split(/_|-| /).map { |part| part.sub(/^\w/) { |c| c.upcase } }.join end - # Find all consecutive nodes satisfying the given {Proc} of a minimum size - # and yield each group. + # Find all consecutive items satisfying the given block of a minimum size, + # yielding each group of consecutive items to the provided block. # # @param items [Array] - # @param min_size [Fixnum] minimum number of consecutive items before - # yielding # @param satisfies [Proc] function that takes an item and returns true/false - def find_consecutive(items, min_size, satisfies) - current = -1 + # @param min_consecutive [Fixnum] minimum number of consecutive items before + # yielding the group + # @yield Passes list of consecutive items all matching the criteria defined + # by the `satisfies` {Proc} to the provided block + # @yieldparam group [Array] List of consecutive items + # @yieldreturn [Boolean] block should return whether item matches criteria + # for inclusion + def for_consecutive_items(items, satisfies, min_consecutive = 2) + current_index = -1 - while (current += 1) < items.count - next unless satisfies[items[current]] + while (current_index += 1) < items.count + next unless satisfies[items[current_index]] - count = count_consecutive(items, current, satisfies) - next unless count >= min_size + count = count_consecutive(items, current_index, &satisfies) + next unless count >= min_consecutive # Yield the chunk of consecutive items - yield items[current...(current + count)] + yield items[current_index...(current_index + count)] - current += count # Skip this patch of consecutive items to find more + current_index += count # Skip this patch of consecutive items to find more end end # Count the number of consecutive items satisfying the given {Proc}. # # @param items [Array] - # @param offset [Fixnum] index to start searching - # @param satisfies [Proc] function to evaluate item with - def count_consecutive(items, offset, satisfies) + # @param offset [Fixnum] index to start searching from + # @yield [item] Passes item to the provided block. + # @yieldparam item [Object] Item to evaluate as matching criteria for + # inclusion + # @yieldreturn [Boolean] whether to include the item + # @return [Integer] + def count_consecutive(items, offset = 0, &block) count = 1 - count += 1 while (offset + count < items.count) && satisfies[items[offset + count]] + count += 1 while (offset + count < items.count) && block.call(items[offset + count]) count end + + # Calls a block of code with a modified set of environment variables, + # restoring them once the code has executed. + def with_environment(env) + old_env = {} + env.each do |var, value| + old_env[var] = ENV[var.to_s] + ENV[var.to_s] = value + end + + yield + ensure + old_env.each { |var, value| ENV[var.to_s] = value } + end end end diff --git a/spec/haml_lint/cli_spec.rb b/spec/haml_lint/cli_spec.rb index 69a7df8c..ad5ed76d 100644 --- a/spec/haml_lint/cli_spec.rb +++ b/spec/haml_lint/cli_spec.rb @@ -122,6 +122,16 @@ it { should == Sysexits::EX_OK } end + context 'when passed the --help flag' do + let(:args) { ['--help'] } + + it 'displays usage information' do + subject + output.should include HamlLint::APP_NAME + output.should include 'Usage' + end + end + context 'when passed the --version flag' do let(:args) { ['--version'] } @@ -136,17 +146,38 @@ end end - context 'when invalid argument is given' do - let(:args) { ['--some-invalid-argument'] } + context 'when a ConfigurationError is raised' do + before do + cli.stub(:act_on_options).and_raise(HamlLint::Exceptions::ConfigurationError) + end - it 'displays message about invalid option' do - subject - output.should =~ /invalid option/i + it { should == Sysexits::EX_CONFIG } + end + + context 'when an InvalidCLIOption error is raised' do + before do + cli.stub(:act_on_options).and_raise(HamlLint::Exceptions::InvalidCLIOption) end it { should == Sysexits::EX_USAGE } end + context 'when an InvalidFilePath error is raised' do + before do + cli.stub(:act_on_options).and_raise(HamlLint::Exceptions::InvalidFilePath) + end + + it { should == Sysexits::EX_NOINPUT } + end + + context 'when a NoLintersError is raised' do + before do + cli.stub(:act_on_options).and_raise(HamlLint::Exceptions::NoLintersError) + end + + it { should == Sysexits::EX_NOINPUT } + end + context 'when an unhandled exception occurs' do let(:backtrace) { %w[file1.rb:1 file2.rb:2] } let(:error_msg) { 'Oops' } diff --git a/spec/haml_lint/configuration_loader_spec.rb b/spec/haml_lint/configuration_loader_spec.rb index d76267da..086e692b 100644 --- a/spec/haml_lint/configuration_loader_spec.rb +++ b/spec/haml_lint/configuration_loader_spec.rb @@ -39,4 +39,97 @@ end end end + + describe '.default_configuration' do + subject { described_class.default_configuration } + + before do + # Ensure cache is cleared + described_class.instance_variable_set(:@default_config, nil) + end + + it 'loads the default config file' do + described_class.should_receive(:load_from_file) + .with(HamlLint::ConfigurationLoader::DEFAULT_CONFIG_PATH) + subject + end + end + + describe '.load_file' do + let(:file_name) { 'config.yml' } + subject { described_class.load_file(file_name) } + + around do |example| + directory { example.run } + end + + context 'with a file that exists' do + before do + File.open(file_name, 'w') { |f| f.write(config_file) } + end + + context 'and is empty' do + let(:config_file) { '' } + + it 'is equivalent to the default configuration' do + subject.should == described_class.default_configuration + end + end + + context 'and is valid' do + let(:config_file) { 'skip_frontmatter: true' } + + it 'loads the custom configuration' do + subject['skip_frontmatter'].should == true + end + + it 'extends the default configuration' do + custom_config = HamlLint::Configuration.new('skip_frontmatter' => true) + + subject.should == + described_class.default_configuration.merge(custom_config) + end + end + + context 'and is invalid' do + let(:config_file) { normalize_indent(<<-CONF) } + linters: + SomeLinter: + invalid + CONF + + it 'raises an error' do + expect { subject }.to raise_error HamlLint::Exceptions::ConfigurationError + end + end + end + + context 'with a file that does not exist' do + it 'raises an error' do + expect { subject }.to raise_error HamlLint::Exceptions::ConfigurationError + end + end + end + + describe '.load_hash' do + subject { described_class.load_hash(hash) } + + context 'when hash is empty' do + let(:hash) { {} } + + it 'is equivalent to the default configuration' do + subject.should == described_class.default_configuration + end + end + + context 'when hash is not empty' do + let(:hash) { { 'skip_frontmatter' => true } } + + it 'extends the default configuration' do + subject.should == + described_class.default_configuration + .merge(HamlLint::Configuration.new(hash)) + end + end + end end diff --git a/spec/haml_lint/configuration_spec.rb b/spec/haml_lint/configuration_spec.rb new file mode 100644 index 00000000..8b98b69e --- /dev/null +++ b/spec/haml_lint/configuration_spec.rb @@ -0,0 +1,63 @@ +require 'spec_helper' + +describe HamlLint::Configuration do + let(:config) { HamlLint::ConfigurationLoader.default_configuration } + + describe '#initialize' do + let(:config) { described_class.new(hash) } + subject { config } + + context 'with an empty hash' do + let(:hash) { {} } + + it 'creates an empty `exclude` section' do + subject['exclude'].should == [] + end + + it 'creates an empty `linters` section' do + subject['linters'].should == {} + end + end + + context 'with a linter with single values in its `include`/`exclude` options' do + let(:hash) do + { + 'linters' => { + 'SomeLinter' => { + 'include' => '**/*.haml', + 'exclude' => '**/*.ignore.haml', + }, + }, + } + end + + it 'converts the `include` value into an array' do + subject['linters']['SomeLinter']['include'].should == ['**/*.haml'] + end + + it 'converts the `exclude` value into an array' do + subject['linters']['SomeLinter']['exclude'].should == ['**/*.ignore.haml'] + end + end + end + + describe '#for_linter' do + subject { config.for_linter(linter) } + + context 'when linter is a Class' do + let(:linter) { HamlLint::Linter::LineLength } + + it 'returns the configuration for the relevant linter' do + subject['max'].should == 80 + end + end + + context 'when linter is a Linter' do + let(:linter) { HamlLint::Linter::LineLength.new(double) } + + it 'returns the configuration for the relevant linter' do + subject['max'].should == 80 + end + end + end +end diff --git a/spec/haml_lint/document_spec.rb b/spec/haml_lint/document_spec.rb new file mode 100644 index 00000000..f9d63d48 --- /dev/null +++ b/spec/haml_lint/document_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe HamlLint::Document do + let(:config) { double } + + before do + config.stub(:[]).with('skip_frontmatter').and_return(false) + end + + describe '#initialize' do + let(:source) { normalize_indent(<<-HAML) } + %head + %title My title + %body + %p My paragraph + HAML + + let(:options) { { config: config } } + + subject { described_class.new(source, options) } + + it 'stores a tree representing the parsed document' do + subject.tree.should be_a HamlLint::Tree::Node + end + + it 'stores the source code' do + subject.source == source + end + + it 'stores the individual lines of source code' do + subject.source_lines == source.split("\n") + end + + context 'when file is explicitly specified' do + let(:options) { super().merge(file: 'my_file.haml') } + + it 'sets the file name' do + subject.file == 'my_file.haml' + end + end + + context 'when file is not specified' do + it 'sets a dummy file name' do + subject.file == HamlLint::Document::STRING_SOURCE + end + end + + context 'when skip_frontmatter is specified in config' do + before do + config.stub(:[]).with('skip_frontmatter').and_return(true) + end + + context 'and the source contains frontmatter' do + let(:source) { "---\nsome frontmatter\n---\n#{super()}" } + + it 'removes the frontmatter' do + subject.source.should_not include '---' + subject.source.should include '%head' + end + end + + context 'and the source does not contain frontmatter' do + it 'leaves the source untouched' do + subject.source == source + end + end + end + end +end diff --git a/spec/haml_lint/haml_visitor_spec.rb b/spec/haml_lint/haml_visitor_spec.rb index e54196a7..4d7200cf 100644 --- a/spec/haml_lint/haml_visitor_spec.rb +++ b/spec/haml_lint/haml_visitor_spec.rb @@ -2,9 +2,15 @@ describe HamlLint::HamlVisitor do describe '#visit' do - let(:parser) { HamlLint::Parser.new(normalize_indent(haml)) } + let(:options) do + { + config: HamlLint::ConfigurationLoader.default_configuration, + } + end + + let(:document) { HamlLint::Document.new(normalize_indent(haml), options) } - before { visitor.visit(parser.tree) } + before { visitor.visit(document.tree) } class TrackingVisitor include HamlLint::HamlVisitor diff --git a/spec/haml_lint/linter/rubocop_spec.rb b/spec/haml_lint/linter/rubocop_spec.rb index 7d1da449..a16384c8 100644 --- a/spec/haml_lint/linter/rubocop_spec.rb +++ b/spec/haml_lint/linter/rubocop_spec.rb @@ -1,25 +1,36 @@ require 'spec_helper' describe HamlLint::Linter::RuboCop do + let!(:rubocop_cli) { spy('rubocop_cli') } + # Need this block before including linter context so that stubbing occurs # before linter is run - before { subject.stub(:lint_file).and_return([offence].compact) } + before do + ::RuboCop::CLI.stub(:new).and_return(rubocop_cli) + HamlLint::OffenseCollector.stub(:offenses) + .and_return([offence].compact) + end include_context 'linter' + let(:offence) { nil } + let(:haml) { <<-HAML } %span To be %span= "or not" %span to be HAML + it 'does not specify the --config flag by default' do + expect(rubocop_cli).to have_received(:run).with(array_excluding('--config')) + end + context 'when RuboCop does not report offences' do - let(:offence) { nil } it { should_not report_lint } end context 'when RuboCop reports offences' do - let(:line) { 3 } + let(:line) { 4 } let(:message) { 'Lint message' } let(:cop_name) { 'Lint/SomeCopName' } @@ -28,7 +39,7 @@ end it 'uses the source map to transform line numbers' do - subject.should report_lint line: 2 + subject.should report_lint line: 3 end context 'and the offence is from an ignored cop' do @@ -36,4 +47,17 @@ it { should_not report_lint } end end + + context 'when the HAML_LINT_RUBOCOP_CONF environment variable is specified' do + around do |example| + HamlLint::Utils.with_environment 'HAML_LINT_RUBOCOP_CONF' => 'some-rubocop.yml' do + example.run + end + end + + it 'specifies the --config flag' do + expect(rubocop_cli) + .to have_received(:run).with(array_including('--config', 'some-rubocop.yml')) + end + end end diff --git a/spec/haml_lint/linter_spec.rb b/spec/haml_lint/linter_spec.rb new file mode 100644 index 00000000..6a615b06 --- /dev/null +++ b/spec/haml_lint/linter_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe HamlLint::Linter do + let(:linter_class) do + Class.new(described_class) do + def visit_root(node) + add_lint(node, 'A lint!') + end + end + end + + let(:config) { double } + let(:linter) { linter_class.new(config) } + + describe '#run' do + let(:options) do + { + config: HamlLint::ConfigurationLoader.default_configuration, + file: 'file.haml', + } + end + + let(:document) { HamlLint::Document.new('%p Hello world', options) } + subject { linter.run(document) } + + it 'returns the reported lints' do + subject.length.should == 1 + end + + context 'when a linter calls parse_ruby' do + let(:linter_class) do + Class.new(described_class) do + attr_reader :parsed_ruby + + def visit_root(_node) + @parsed_ruby = parse_ruby('puts') + end + end + end + + it 'parses the ruby' do + subject + linter.parsed_ruby.type.should == :send + end + end + end + + describe '#name' do + subject { linter.name } + + before do + linter.class.stub(:name).and_return('HamlLint::Linter::SomeLinterName') + end + + it { should == 'SomeLinterName' } + end +end diff --git a/spec/haml_lint/options_spec.rb b/spec/haml_lint/options_spec.rb index 4814c745..278757bd 100644 --- a/spec/haml_lint/options_spec.rb +++ b/spec/haml_lint/options_spec.rb @@ -39,6 +39,32 @@ end end + context 'with a reporter option' do + context 'for a reporter that exists' do + let(:args) { %w[--reporter Json] } + + it 'sets the `reporter` option' do + subject.should include reporter: HamlLint::Reporter::JsonReporter + end + end + + context 'for a reporter that exists when capitalized' do + let(:args) { %w[--reporter json] } + + it 'sets the `reporter` option' do + subject.should include reporter: HamlLint::Reporter::JsonReporter + end + end + + context 'for a reporter that does not exist' do + let(:args) { %w[--reporter NonExistent] } + + it 'raises an error' do + expect { subject }.to raise_error HamlLint::Exceptions::InvalidCLIOption + end + end + end + context 'with the help option' do let(:args) { ['--help'] } diff --git a/spec/haml_lint/parser_spec.rb b/spec/haml_lint/parser_spec.rb deleted file mode 100644 index dc65a47b..00000000 --- a/spec/haml_lint/parser_spec.rb +++ /dev/null @@ -1,61 +0,0 @@ -require 'spec_helper' - -describe HamlLint::Parser do - context 'when skip_frontmatter is true' do - let(:parser) { HamlLint::Parser.new(haml, 'skip_frontmatter' => true) } - - let(:haml) { normalize_indent(<<-HAML) } - --- - :key: value - --- - %tag - Some non-inline text - - 'some code' - HAML - - it 'excludes the frontmatter' do - expect(parser.contents).to eq(normalize_indent(<<-CONTENT)) - %tag - Some non-inline text - - 'some code' - CONTENT - end - - context 'when haml has --- as content' do - let(:haml) { normalize_indent(<<-HAML) } - --- - :key: value - --- - %tag - Some non-inline text - - 'some code' - --- - HAML - - it 'is not greedy' do - expect(parser.contents).to eq(normalize_indent(<<-CONTENT)) - %tag - Some non-inline text - - 'some code' - --- - CONTENT - end - end - end - - context 'when skip_frontmatter is false' do - let(:parser) { HamlLint::Parser.new(haml, 'skip_frontmatter' => false) } - let(:haml) { normalize_indent(<<-HAML) } - --- - :key: value - --- - %tag - Some non-inline text - - 'some code' - HAML - - it 'raises a HAML error' do - expect { parser }.to raise_error(/invalid filter/i) - end - end -end diff --git a/spec/haml_lint/rake_task_spec.rb b/spec/haml_lint/rake_task_spec.rb index 94e70d34..d5fbf23f 100644 --- a/spec/haml_lint/rake_task_spec.rb +++ b/spec/haml_lint/rake_task_spec.rb @@ -23,15 +23,15 @@ def run_task end end - context 'when HAML document is valid' do - let(:haml) { '%tag' } + context 'when Haml document is valid' do + let(:haml) { '%p Hello world' } it 'executes without error' do expect { run_task }.not_to raise_error end end - context 'when HAML document is invalid' do + context 'when Haml document is invalid' do let(:haml) { "%tag\n %foo\n %bar" } it 'raises an error' do diff --git a/spec/haml_lint/reporter/default_reporter_spec.rb b/spec/haml_lint/reporter/default_reporter_spec.rb index dad7beca..9558db99 100644 --- a/spec/haml_lint/reporter/default_reporter_spec.rb +++ b/spec/haml_lint/reporter/default_reporter_spec.rb @@ -1,16 +1,14 @@ require 'spec_helper' describe HamlLint::Reporter::DefaultReporter do - subject { HamlLint::Reporter::DefaultReporter.new(lints) } - - describe '#report_lints' do + describe '#display_report' do let(:io) { StringIO.new } let(:output) { io.string } let(:logger) { HamlLint::Logger.new(io) } let(:report) { HamlLint::Report.new(lints, []) } - let(:reporter) { described_class.new(logger, report) } + let(:reporter) { described_class.new(logger) } - subject { reporter.report_lints } + subject { reporter.display_report(report) } context 'when there are no lints' do let(:lints) { [] } @@ -26,10 +24,11 @@ let(:lines) { [502, 724] } let(:descriptions) { ['Description of lint 1', 'Description of lint 2'] } let(:severities) { [:warning] * 2 } + let(:linter) { double(name: 'SomeLinter') } let(:lints) do filenames.each_with_index.map do |filename, index| - HamlLint::Lint.new(nil, filename, lines[index], descriptions[index], severities[index]) + HamlLint::Lint.new(linter, filename, lines[index], descriptions[index], severities[index]) end end @@ -83,6 +82,17 @@ end end end + + context 'when lint has no associated linter' do + let(:linter) { nil } + + it 'prints the description for each lint' do + subject + descriptions.each do |description| + output.scan(/#{description}/).count.should == 1 + end + end + end end end end diff --git a/spec/haml_lint/reporter/json_reporter_spec.rb b/spec/haml_lint/reporter/json_reporter_spec.rb index cf198d10..fabb5029 100644 --- a/spec/haml_lint/reporter/json_reporter_spec.rb +++ b/spec/haml_lint/reporter/json_reporter_spec.rb @@ -1,21 +1,19 @@ require 'spec_helper' describe HamlLint::Reporter::JsonReporter do - subject { HamlLint::Reporter::JsonReporter.new(lints) } - - describe '#report_lints' do + describe '#display_report' do let(:io) { StringIO.new } let(:output) { JSON.parse(io.string) } let(:logger) { HamlLint::Logger.new(io) } let(:report) { HamlLint::Report.new(lints, []) } - let(:reporter) { described_class.new(logger, report) } + let(:reporter) { described_class.new(logger) } - subject { reporter.report_lints } + subject { reporter.display_report(report) } shared_examples_for 'output format specification' do it 'matches the output specification' do subject - output['metadata']['hamllint_version'].should_not be_empty + output['metadata']['haml_lint_version'].should_not be_empty output['metadata']['ruby_engine'].should eq RUBY_ENGINE output['metadata']['ruby_patchlevel'].should eq RUBY_PATCHLEVEL.to_s output['metadata']['ruby_platform'].should eq RUBY_PLATFORM.to_s diff --git a/spec/haml_lint/script_extractor_spec.rb b/spec/haml_lint/ruby_extractor_spec.rb similarity index 60% rename from spec/haml_lint/script_extractor_spec.rb rename to spec/haml_lint/ruby_extractor_spec.rb index 08c6b6d6..ea6474e5 100644 --- a/spec/haml_lint/script_extractor_spec.rb +++ b/spec/haml_lint/ruby_extractor_spec.rb @@ -1,15 +1,22 @@ require 'spec_helper' -describe HamlLint::ScriptExtractor do - let(:parser) { HamlLint::Parser.new(normalize_indent(haml)) } - let(:extractor) { described_class.new(parser) } +describe HamlLint::RubyExtractor do + let(:extractor) { described_class.new } describe '#extract' do - subject { extractor.extract } + let(:options) do + { + config: HamlLint::ConfigurationLoader.default_configuration, + } + end + + let(:tree) { HamlLint::Document.new(normalize_indent(haml), options) } + subject { extractor.extract(tree) } context 'with an empty HAML document' do let(:haml) { '' } - it { should == '' } + its(:source) { should == '' } + its(:source_map) { should == {} } end context 'with plain text' do @@ -17,7 +24,8 @@ Hello world HAML - it { should == 'puts' } + its(:source) { should == 'puts' } + its(:source_map) { should == { 1 => 1 } } end context 'with multiple lines of plain text' do @@ -26,7 +34,8 @@ how are you? HAML - it { should == "puts\nputs" } + its(:source) { should == "puts\nputs" } + its(:source_map) { should == { 1 => 1, 2 => 2 } } end context 'with only tags with text content' do @@ -37,7 +46,8 @@ %b Ipsum HAML - it { should == "puts # h1\nputs # p\nputs\nputs # b" } + its(:source) { should == "puts # h1\nputs # p\nputs\nputs # b" } + its(:source_map) { should == { 1 => 1, 2 => 2, 3 => 3, 4 => 4 } } end context 'with a silent script node' do @@ -45,7 +55,8 @@ - silent_script HAML - it { should == 'silent_script' } + its(:source) { should == 'silent_script' } + its(:source_map) { should == { 1 => 1 } } end context 'with a script node' do @@ -53,7 +64,8 @@ = script HAML - it { should == 'script' } + its(:source) { should == 'script' } + its(:source_map) { should == { 1 => 1 } } end context 'with a script node that spans multiple lines' do @@ -63,7 +75,8 @@ class: 'button' HAML - it { should == "link_to 'Link', path, class: 'button'" } + its(:source) { should == "link_to 'Link', path, class: 'button'" } + its(:source_map) { should == { 1 => 1 } } end context 'with a tag containing a silent script node' do @@ -72,7 +85,8 @@ - script HAML - it { should == "puts # tag\nscript" } + its(:source) { should == "puts # tag\nscript" } + its(:source_map) { should == { 1 => 1, 2 => 2 } } end context 'with a tag containing a script node' do @@ -81,7 +95,8 @@ = script HAML - it { should == "puts # tag\nscript" } + its(:source) { should == "puts # tag\nscript" } + its(:source_map) { should == { 1 => 1, 2 => 2 } } end context 'with a tag containing inline script' do @@ -89,7 +104,8 @@ %tag= script HAML - it { should == "puts # tag\nscript" } + its(:source) { should == "puts # tag\nscript" } + its(:source_map) { should == { 1 => 1, 2 => 1 } } end context 'with a tag with hash attributes' do @@ -97,7 +113,8 @@ %tag{ one: 1, two: 2, 'three' => some_method } HAML - it { should == "{}.merge(one: 1, two: 2, 'three' => some_method)\nputs # tag" } + its(:source) { should == "{}.merge(one: 1, two: 2, 'three' => some_method)\nputs # tag" } + its(:source_map) { should == { 1 => 1, 2 => 1 } } end context 'with a tag with hash attributes with hashrockets and questionable spacing' do @@ -105,9 +122,8 @@ %tag.class_one.class_two#with_an_id{:type=>'checkbox', 'special' => :true } HAML - it 'includes the hash attribute source for rubocop inspection' do - should == "{:type=>'checkbox', 'special' => :true }\nputs # tag" - end + its(:source) { should == "{:type=>'checkbox', 'special' => :true }\nputs # tag" } + its(:source_map) { should == { 1 => 1, 2 => 1 } } end context 'with a tag with mixed-style hash attributes' do @@ -115,7 +131,8 @@ %tag.class_one.class_two#with_an_id{ :type=>'checkbox', special: 'true' } HAML - it { should == "{}.merge(:type=>'checkbox', special: 'true')\nputs # tag" } + its(:source) { should == "{}.merge(:type=>'checkbox', special: 'true')\nputs # tag" } + its(:source_map) { should == { 1 => 1, 2 => 1 } } end context 'with a tag with hash attributes with a method call' do @@ -123,7 +140,8 @@ %tag{ tag_options_method } HAML - it { should == "{}.merge(tag_options_method)\nputs # tag" } + its(:source) { should == "{}.merge(tag_options_method)\nputs # tag" } + its(:source_map) { should == { 1 => 1, 2 => 1 } } end context 'with a tag with HTML-style attributes' do @@ -131,7 +149,12 @@ %tag(one=1 two=2 three=some_method) HAML - it { should == "{}.merge({\"one\" => 1,\"two\" => 2,\"three\" => some_method,})\nputs # tag" } + its(:source) { should == normalize_indent(<<-RUBY).rstrip } + {}.merge({\"one\" => 1,\"two\" => 2,\"three\" => some_method,}) + puts # tag + RUBY + + its(:source_map) { should == { 1 => 1, 2 => 1 } } end context 'with a tag with hash attributes and inline script' do @@ -139,11 +162,13 @@ %tag{ one: 1 }= script HAML - it { should == normalize_indent(<<-RUBY).rstrip } + its(:source) { should == normalize_indent(<<-RUBY).rstrip } {}.merge(one: 1) puts # tag script RUBY + + its(:source_map) { should == { 1 => 1, 2 => 1, 3 => 1 } } end context 'with a tag with hash attributes that span multiple lines' do @@ -153,7 +178,8 @@ 'three' => 3 } HAML - it { should == "{}.merge(one: 1, two: 2, 'three' => 3)\nputs # tag" } + its(:source) { should == "{}.merge(one: 1, two: 2, 'three' => 3)\nputs # tag" } + its(:source_map) { should == { 1 => 1, 2 => 1 } } end context 'with a tag with 1.8-style hash attributes of string key/values' do @@ -161,7 +187,8 @@ %tag{ 'one' => '1', 'two' => '2' } HAML - it { should == "{ 'one' => '1', 'two' => '2' }\nputs # tag" } + its(:source) { should == "{ 'one' => '1', 'two' => '2' }\nputs # tag" } + its(:source_map) { should == { 1 => 1, 2 => 1 } } context 'that span multiple lines' do let(:haml) { <<-HAML } @@ -169,7 +196,8 @@ 'two' => '2' } HAML - it { should == "{ 'one' => '1', 'two' => '2' }\nputs # div" } + its(:source) { should == "{ 'one' => '1', 'two' => '2' }\nputs # div" } + its(:source_map) { should == { 1 => 1, 2 => 1 } } end end @@ -183,7 +211,7 @@ - script_three HAML - it { should == normalize_indent(<<-RUBY).rstrip } + its(:source) { should == normalize_indent(<<-RUBY).rstrip } if condition script_one elsif condition_two @@ -192,6 +220,8 @@ script_three end RUBY + + its(:source_map) { should == { 1 => 1, 2 => 2, 3 => 3, 4 => 4, 5 => 5, 6 => 6, 7 => 1 } } end context 'with an anonymous block' do @@ -200,11 +230,13 @@ = script HAML - it { should == normalize_indent(<<-RUBY).rstrip } + its(:source) { should == normalize_indent(<<-RUBY).rstrip } link_to path do script end RUBY + + its(:source_map) { should == { 1 => 1, 2 => 2, 3 => 1 } } end context 'with a for loop' do @@ -213,11 +245,13 @@ = value HAML - it { should == normalize_indent(<<-RUBY).rstrip } + its(:source) { should == normalize_indent(<<-RUBY).rstrip } for value in list value end RUBY + + its(:source_map) { should == { 1 => 1, 2 => 2, 3 => 1 } } end context 'with a while loop' do @@ -227,12 +261,14 @@ - value += 1 HAML - it { should == normalize_indent(<<-RUBY).rstrip } + its(:source) { should == normalize_indent(<<-RUBY).rstrip } while value < 10 value value += 1 end RUBY + + its(:source_map) { should == { 1 => 1, 2 => 2, 3 => 3, 4 => 1 } } end context 'with a Ruby filter' do @@ -244,12 +280,14 @@ end HAML - it { should == normalize_indent(<<-RUBY).rstrip } + its(:source) { should == normalize_indent(<<-RUBY).rstrip } method_one if condition method_two end RUBY + + its(:source_map) { should == { 1 => 2, 2 => 3, 3 => 4, 4 => 5 } } end context 'with a Ruby filter containing block keywords' do @@ -262,7 +300,7 @@ end HAML - it { should == normalize_indent(<<-RUBY).rstrip } + its(:source) { should == normalize_indent(<<-RUBY).rstrip } if condition do_something else @@ -270,6 +308,8 @@ end RUBY + its(:source_map) { should == { 1 => 2, 2 => 3, 3 => 4, 4 => 5, 5 => 6 } } + context 'and the filter is nested' do let(:haml) { <<-HAML } - something do @@ -281,7 +321,7 @@ end HAML - it { should == normalize_indent(<<-RUBY).rstrip } + its(:source) { should == normalize_indent(<<-RUBY).rstrip } something do if condition do_something @@ -290,6 +330,8 @@ end end RUBY + + its(:source_map) { should == { 1 => 1, 2 => 3, 3 => 4, 4 => 5, 5 => 6, 6 => 7, 7 => 1 } } end end @@ -300,11 +342,13 @@ Some more text \#{some_other_method} with interpolation. HAML - it { should == normalize_indent(<<-RUBY).rstrip } + its(:source) { should == normalize_indent(<<-RUBY).rstrip } puts some_method some_other_method RUBY + + its(:source_map) { should == { 1 => 1, 2 => 1, 3 => 1 } } end context 'with a filter with interpolated values containing quotes' do @@ -314,11 +358,13 @@ Some text \#{some_other_method('world')} HAML - it { should == normalize_indent(<<-RUBY).rstrip } + its(:source) { should == normalize_indent(<<-RUBY).rstrip } puts some_method("hello") some_other_method('world') RUBY + + its(:source_map) { should == { 1 => 1, 2 => 1, 3 => 1 } } end context 'with a filter with interpolated values spanning multiple lines' do @@ -330,11 +376,13 @@ # TODO: Figure out if it's worth normalizing indentation for the generated # code in this interpolated context - it { should == normalize_indent(<<-RUBY).rstrip } + its(:source) { should == normalize_indent(<<-RUBY).rstrip } puts some_method('hello', 'world') RUBY + + its(:source_map) { should == { 1 => 1, 2 => 1, 3 => 1 } } end context 'with an if/else block containing only filters' do @@ -347,13 +395,15 @@ Some other text HAML - it { should == normalize_indent(<<-RUBY).rstrip } + its(:source) { should == normalize_indent(<<-RUBY).rstrip } if condition puts else puts end RUBY + + its(:source_map) { should == { 1 => 1, 2 => 2, 3 => 4, 4 => 5, 5 => 1 } } end end end diff --git a/spec/haml_lint/runner_spec.rb b/spec/haml_lint/runner_spec.rb index 6fb1916b..5a35fa74 100644 --- a/spec/haml_lint/runner_spec.rb +++ b/spec/haml_lint/runner_spec.rb @@ -2,27 +2,14 @@ describe HamlLint::Runner do let(:options) { {} } - let(:runner) { HamlLint::Runner.new } - let(:config) do - HamlLint::ConfigurationLoader.load_hash( - 'linters' => { - 'FakeLinter1' => { 'enabled' => true }, - 'FakeLinter2' => { 'enabled' => true }, - }, - ) - end - - class FakeLinter1 < HamlLint::Linter; include HamlLint::LinterRegistry; end - class FakeLinter2 < HamlLint::Linter; include HamlLint::LinterRegistry; end + let(:runner) { described_class.new } before do - HamlLint::ConfigurationLoader.stub(:load_applicable_config).and_return(config) - HamlLint::LinterRegistry.stub(:linters).and_return([FakeLinter1, FakeLinter2]) runner.stub(:extract_applicable_files).and_return(files) end describe '#run' do - let(:files) { %w[file1.haml file2.haml] } + let(:files) { %w[file1.slim file2.slim] } let(:mock_linter) { double('linter', lints: [], name: 'Blah') } let(:options) do @@ -34,80 +21,14 @@ class FakeLinter2 < HamlLint::Linter; include HamlLint::LinterRegistry; end subject { runner.run(options) } before do - runner.stub(:find_lints).and_return([]) + runner.stub(:collect_lints).and_return([]) end it 'searches for lints in each file' do - runner.should_receive(:find_lints).exactly(files.size).times + runner.should_receive(:collect_lints).exactly(files.size).times subject end - context 'when the :excluded_linters option is specified' do - let(:options) { super().merge(excluded_linters: ['FakeLinter2']) } - - it 'does not instantiate the excluded linters' do - FakeLinter2.should_not_receive(:new) - subject - end - - it 'instantiates all other linters' do - FakeLinter1.should_receive(:new).and_return(mock_linter) - subject - end - end - - context 'when the :included_linters option is specified' do - let(:options) { { included_linters: ['FakeLinter1'] } } - - it 'includes only the specified linter' do - FakeLinter1.should_receive(:new).and_return(mock_linter) - FakeLinter2.should_not_receive(:new) - subject - end - end - - context 'when :include_linters and :exclude_linters are specified' do - let(:options) do - { - included_linters: %w[FakeLinter1 FakeLinter2], - excluded_linters: ['FakeLinter2'], - } - end - - it 'does not instantiate the excluded linters' do - FakeLinter2.should_not_receive(:new) - subject - end - - it 'instantiates the included linters' do - FakeLinter1.should_receive(:new).and_return(mock_linter) - subject - end - end - - context 'when neither included or excluded linters is specified' do - let(:options) { {} } - - it 'instantiates all registered linters' do - FakeLinter1.should_receive(:new).and_return(mock_linter) - FakeLinter2.should_receive(:new).and_return(mock_linter) - subject - end - end - - context 'when all linters are excluded' do - let(:options) do - { - included_linters: %w[FakeLinter1 FakeLinter2], - excluded_linters: %w[FakeLinter1 FakeLinter2], - } - end - - it 'raises an error' do - expect { subject }.to raise_error HamlLint::Exceptions::NoLintersError - end - end - context 'when :config_file option is specified' do let(:options) { { config_file: 'some-config.yml' } } let(:config) { double('config') } @@ -121,5 +42,22 @@ class FakeLinter2 < HamlLint::Linter; include HamlLint::LinterRegistry; end subject end end + + context 'when `exclude` global config option specifies a list of patterns' do + let(:options) { { config: config, files: files } } + let(:config) { HamlLint::Configuration.new(config_hash) } + let(:config_hash) { { 'exclude' => 'exclude-this-file.slim' } } + + before do + runner.stub(:extract_applicable_files).and_call_original + end + + it 'passes the global exclude patterns to the FileFinder' do + HamlLint::FileFinder.any_instance.should_receive(:find) + .with(files, ['exclude-this-file.slim']) + .and_return([]) + subject + end + end end end diff --git a/spec/haml_lint/tree/haml_comment_node_spec.rb b/spec/haml_lint/tree/haml_comment_node_spec.rb index 161b231e..3c184947 100644 --- a/spec/haml_lint/tree/haml_comment_node_spec.rb +++ b/spec/haml_lint/tree/haml_comment_node_spec.rb @@ -1,8 +1,14 @@ require 'spec_helper' describe HamlLint::Tree::HamlCommentNode do - let(:parser) { HamlLint::Parser.new(normalize_indent(haml)) } - subject { parser.tree.find { |node| node.type == :haml_comment } } + let(:options) do + { + config: HamlLint::ConfigurationLoader.default_configuration, + } + end + + let(:document) { HamlLint::Document.new(normalize_indent(haml), options) } + subject { document.tree.find { |node| node.type == :haml_comment } } describe '#text' do subject { super().text } diff --git a/spec/haml_lint/tree/node_spec.rb b/spec/haml_lint/tree/node_spec.rb index 6a2ec585..c073a214 100644 --- a/spec/haml_lint/tree/node_spec.rb +++ b/spec/haml_lint/tree/node_spec.rb @@ -1,10 +1,16 @@ require 'spec_helper' describe HamlLint::Tree::Node do - let(:parser) { HamlLint::Parser.new(normalize_indent(haml)) } + let(:options) do + { + config: HamlLint::ConfigurationLoader.default_configuration, + } + end + + let(:document) { HamlLint::Document.new(normalize_indent(haml), options) } describe '#find' do - subject { parser.tree.find(&matcher) } + subject { document.tree.find(&matcher) } let(:matcher) { ->(node) { node.type == :haml_comment } } context 'when there are no nodes' do @@ -43,7 +49,7 @@ end describe '#source_code' do - subject { parser.tree.find { |node| node.type == :tag }.source_code } + subject { document.tree.find { |node| node.type == :tag }.source_code } context 'when node in the middle of the document' do let(:haml) { <<-HAML } @@ -116,10 +122,10 @@ end describe '#successor' do - subject { parser.tree.find { |node| node.type == :haml_comment }.successor } + subject { document.tree.find { |node| node.type == :haml_comment }.successor } context 'when finding the successor of the root node' do - subject { parser.tree.successor } + subject { document.tree.successor } let(:haml) { '-# Dummy node' } it { should be_nil } diff --git a/spec/haml_lint/tree/tag_node_spec.rb b/spec/haml_lint/tree/tag_node_spec.rb index 88adf911..d82b8944 100644 --- a/spec/haml_lint/tree/tag_node_spec.rb +++ b/spec/haml_lint/tree/tag_node_spec.rb @@ -1,8 +1,14 @@ require 'spec_helper' describe HamlLint::Tree::TagNode do - let(:parser) { HamlLint::Parser.new(normalize_indent(haml)) } - let(:tag_node) { parser.tree.find { |node| node.type == :tag && node.tag_name == tag_name } } + let(:options) do + { + config: HamlLint::ConfigurationLoader.default_configuration, + } + end + + let(:document) { HamlLint::Document.new(normalize_indent(haml), options) } + let(:tag_node) { document.tree.find { |node| node.type == :tag && node.tag_name == tag_name } } let(:tag_name) { 'my_tag' } describe '#has_hash_attribute?' do diff --git a/spec/haml_lint/utils_spec.rb b/spec/haml_lint/utils_spec.rb new file mode 100644 index 00000000..89870716 --- /dev/null +++ b/spec/haml_lint/utils_spec.rb @@ -0,0 +1,160 @@ +require 'spec_helper' +require 'securerandom' + +describe HamlLint::Utils do + describe 'any_glob_matches?' do + let(:file) { 'match-file.txt' } + subject { described_class.any_glob_matches?(globs, file) } + + context 'when a single matching glob is given' do + let(:globs) { 'match-*.txt' } + + it { should == true } + end + + context 'when a single non-matching glob is given' do + let(:globs) { 'other-*.txt' } + + it { should == false } + end + + context 'when a list of globs is given' do + context 'and none of them match' do + let(:globs) { ['who.txt', 'nope*.txt', 'nomatch-*.txt'] } + + it { should == false } + end + + context 'and one of them match' do + let(:globs) { ['who.txt', 'nope*.txt', 'match-*.txt'] } + + it { should == true } + end + end + end + + describe '.for_consecutive_items' do + let(:min_items) { 2 } + let(:matches_proc) { ->(item) { item } } + + subject do + described_class.for_consecutive_items( + items, + ->(item) { item }, + min_items, + ) + end + + context 'with an empty list' do + let(:items) { [] } + + it 'does not yield' do + expect do |block| + described_class.for_consecutive_items(items, matches_proc, min_items, &block) + end.not_to yield_control + end + end + + context 'with a list with one item' do + let(:items) { [false] } + + it 'does not yield' do + expect do |block| + described_class.for_consecutive_items(items, matches_proc, min_items, &block) + end.not_to yield_control + end + end + + context 'with a list with no consecutive items' do + let(:items) { [true, false] } + + it 'does not yield' do + expect do |block| + described_class.for_consecutive_items(items, matches_proc, min_items, &block) + end.not_to yield_control + end + end + + context 'with a list with one group of consecutive items' do + let(:items) { [false, true, true, false] } + + it 'yields the single group' do + expect do |block| + described_class.for_consecutive_items(items, matches_proc, min_items, &block) + end.to yield_successive_args([true, true]) + end + + context 'and the group is under the required minimum size' do + let(:min_items) { 3 } + + it 'does not yield' do + expect do |block| + described_class.for_consecutive_items(items, matches_proc, min_items, &block) + end.not_to yield_control + end + end + end + + context 'with a list with two groups of consecutive items' do + let(:items) { [false, true, true, false, true, true, true] } + + it 'yields both groups' do + expect do |block| + described_class.for_consecutive_items(items, matches_proc, min_items, &block) + end.to yield_successive_args([true, true], [true, true, true]) + end + + context 'and one of the groups is under the required minimum size' do + let(:min_items) { 3 } + + it 'yields only the larger group' do + expect do |block| + described_class.for_consecutive_items(items, matches_proc, min_items, &block) + end.to yield_successive_args([true, true, true]) + end + end + end + end + + describe '.with_environment' do + let(:var_name) { "SLIM_LINT_TEST_VAR_#{SecureRandom.hex}" } + + shared_examples_for 'with_environment' do + it 'sets the value of the variable within the block' do + described_class.with_environment var_name => 'modified_value' do + ENV[var_name].should == 'modified_value' + end + end + end + + context 'when setting an environment variable that was not already set' do + it_should_behave_like 'with_environment' + + it 'deletes the value once the block has exited' do + described_class.with_environment var_name => 'modified_value' do + # Do something... + end + + ENV[var_name].should be_nil + end + end + + context 'when setting an environment variable that was already set' do + around do |example| + ENV[var_name] = 'previous_value' + example.run + ENV.delete(var_name) + end + + it_should_behave_like 'with_environment' + + it 'restores the old value once the block has exited' do + described_class.with_environment var_name => 'modified_value' do + # Do something... + end + + ENV[var_name].should == 'previous_value' + end + end + end +end diff --git a/spec/support/matchers/array_excluding.rb b/spec/support/matchers/array_excluding.rb new file mode 100644 index 00000000..8d2b0d94 --- /dev/null +++ b/spec/support/matchers/array_excluding.rb @@ -0,0 +1,5 @@ +# Define matcher which determines if an array does NOT include any of a list of +# items. +# +# Unclear why this isn't part of the core, but it's useful. +RSpec::Matchers.define_negated_matcher :array_excluding, :include diff --git a/spec/support/shared_linter_context.rb b/spec/support/shared_linter_context.rb index 29be36ae..218c3c8b 100644 --- a/spec/support/shared_linter_context.rb +++ b/spec/support/shared_linter_context.rb @@ -2,14 +2,17 @@ # variable defined via `let` and normalizing it and running the linter against # it, allowing specs to simply specify whether a lint was reported. shared_context 'linter' do - let(:config) do - HamlLint::ConfigurationLoader.default_configuration - .for_linter(described_class) + let(:options) do + { + config: HamlLint::ConfigurationLoader.default_configuration, + } end + let(:config) { options[:config].for_linter(described_class) } + subject { described_class.new(config) } before do - subject.run(HamlLint::Parser.new(normalize_indent(haml))) + subject.run(HamlLint::Document.new(normalize_indent(haml), options)) end end