From d5541a66fc79ce609892e1c0965e9ca337ee75d3 Mon Sep 17 00:00:00 2001 From: Paul Sadauskas Date: Sun, 17 Jun 2018 16:14:59 -0600 Subject: [PATCH 1/2] Add support for --stdin-filename When other tools, like automatic linters within editors, run a sample through reek, they typically do so via stdin. However, if there are reek config exceptions to apply to that file, then reek has no way of knowing what the original filename was, triggering false positives. This adds an `--stdin-filename` CLI option that takes the original filename, and passes it through to the Examiner and SourceCode file to use as the `origin` attribute, allowing the configuration directives to apply. --- .../command_line_interface/options.feature | 1 + features/command_line_interface/stdin.feature | 29 +++++++++++++++++++ features/step_definitions/reek_steps.rb | 4 +++ lib/reek/cli/command/report_command.rb | 1 + lib/reek/cli/options.rb | 8 ++++- lib/reek/examiner.rb | 4 ++- lib/reek/source/source_code.rb | 7 +++-- spec/reek/cli/application_spec.rb | 12 ++++++++ 8 files changed, 61 insertions(+), 5 deletions(-) diff --git a/features/command_line_interface/options.feature b/features/command_line_interface/options.feature index 6b4f826f6..acc22200b 100644 --- a/features/command_line_interface/options.feature +++ b/features/command_line_interface/options.feature @@ -44,6 +44,7 @@ Feature: Reek can be controlled using command-line options --smell SMELL Only look for a specific smell. Call it like this: reek --smell MissingSafeMethod source.rb Check out https://github.com/troessner/reek/blob/v4.8.1/docs/Code-Smells.md for a list of smells + --stdin-filename FILE When passing code in via pipe, assume this filename when checking file or directory rules in the config. Generate a todo list: -t, --todo Generate a todo list diff --git a/features/command_line_interface/stdin.feature b/features/command_line_interface/stdin.feature index a2f972c50..18359ce67 100644 --- a/features/command_line_interface/stdin.feature +++ b/features/command_line_interface/stdin.feature @@ -41,3 +41,32 @@ Feature: Reek reads from $stdin when no files are given [2]:Syntax: This file has unexpected token $end [1]:Syntax: This file has unexpected token tEQL """ + + Scenario: providing a filename to use for the config to match against + Given a file named "web_app/config.reek" with: + """ + --- + directories: + "web_app/app/controllers": + IrresponsibleModule: + enabled: false + NestedIterators: + enabled: false + InstanceVariableAssumption: + enabled: false + """ + When I pass a stdin to reek --config web_app/config.reek --stdin-filename web_app/app/controllers/users_controller with: + """ + class UsersController < ApplicationController + def show + respond_with do |format| + format.json { |json| @user.to_custom_json } + format.xml { |xml| @user.to_fancy_xml } + end + end + end + """ + Then it succeeds + + + diff --git a/features/step_definitions/reek_steps.rb b/features/step_definitions/reek_steps.rb index 91e06cdc0..787a25a59 100644 --- a/features/step_definitions/reek_steps.rb +++ b/features/step_definitions/reek_steps.rb @@ -6,6 +6,10 @@ reek_with_pipe(stdin, args) end +When /^I pass a stdin to reek *(.*) with:$/ do |args, stdin| + reek_with_pipe(stdin, args) +end + Then /^it reports nothing$/ do expect(last_command_started).to have_output_on_stdout('') end diff --git a/lib/reek/cli/command/report_command.rb b/lib/reek/cli/command/report_command.rb index 0395855b0..2fdd529b0 100644 --- a/lib/reek/cli/command/report_command.rb +++ b/lib/reek/cli/command/report_command.rb @@ -24,6 +24,7 @@ def execute def populate_reporter_with_smells sources.each do |source| reporter.add_examiner Examiner.new(source, + stdin_filename: options.stdin_filename, filter_by_smells: smell_names, configuration: configuration, error_handler: LoggingErrorHandler.new) diff --git a/lib/reek/cli/options.rb b/lib/reek/cli/options.rb index 5a04ff8fa..a1a287638 100644 --- a/lib/reek/cli/options.rb +++ b/lib/reek/cli/options.rb @@ -28,6 +28,7 @@ class Options :show_empty, :show_links, :sorting, + :stdin_filename, :success_exit_code, :failure_exit_code, :generate_todo_list, @@ -98,7 +99,7 @@ def set_banner BANNER end - # :reek:TooManyStatements { max_statements: 6 } + # :reek:TooManyStatements { max_statements: 7 } def set_configuration_options parser.separator 'Configuration:' parser.on('-c', '--config FILE', 'Read configuration options from FILE') do |file| @@ -111,6 +112,11 @@ def set_configuration_options 'for a list of smells') do |smell| smells_to_detect << smell end + parser.on('--stdin-filename FILE', + 'When passing code in via pipe, assume this filename when '\ + 'checking file or directory rules in the config.') do |file| + self.stdin_filename = Pathname.new(file) + end end def set_generate_todo_list_options diff --git a/lib/reek/examiner.rb b/lib/reek/examiner.rb index 584e0ac1e..ee6f9c190 100644 --- a/lib/reek/examiner.rb +++ b/lib/reek/examiner.rb @@ -33,12 +33,14 @@ def handle(_exception) # The configuration for this Examiner. # # @public + # :reek:LongParameterList { max_params: 6 } def initialize(source, + stdin_filename: nil, filter_by_smells: [], configuration: Configuration::AppConfiguration.default, detector_repository_class: DetectorRepository, error_handler: NullHandler.new) - @source = Source::SourceCode.from(source) + @source = Source::SourceCode.from(source, filename: stdin_filename) @smell_types = detector_repository_class.eligible_smell_types(filter_by_smells) @detector_repository = detector_repository_class.new(smell_types: @smell_types, configuration: configuration.directive_for(description)) diff --git a/lib/reek/source/source_code.rb b/lib/reek/source/source_code.rb index 8801c4155..b0d0341b9 100644 --- a/lib/reek/source/source_code.rb +++ b/lib/reek/source/source_code.rb @@ -59,12 +59,13 @@ def initialize(code:, origin:, parser: self.class.default_parser) # # @return an instance of SourceCode # :reek:DuplicateMethodCall { max_calls: 2 } - def self.from(source) + # :reek:ControlParameter + def self.from(source, filename: nil) case source when File then new(code: source.read, origin: source.path) - when IO then new(code: source.readlines.join, origin: IO_IDENTIFIER) + when IO then new(code: source.readlines.join, origin: filename || IO_IDENTIFIER) when Pathname then new(code: source.read, origin: source.to_s) - when String then new(code: source, origin: STRING_IDENTIFIER) + when String then new(code: source, origin: filename || STRING_IDENTIFIER) end end diff --git a/spec/reek/cli/application_spec.rb b/spec/reek/cli/application_spec.rb index 5ed5d30cf..2a7ea6bb0 100644 --- a/spec/reek/cli/application_spec.rb +++ b/spec/reek/cli/application_spec.rb @@ -44,6 +44,18 @@ configuration: Reek::Configuration::AppConfiguration, options: Reek::CLI::Options) end + + context 'when a stdin filename is provided' do + let(:app) { described_class.new ['--stdin-filename', 'foo.rb'] } + + it 'assumes that filename' do + app.execute + expect(Reek::CLI::Command::ReportCommand).to have_received(:new). + with(sources: [$stdin], + configuration: Reek::Configuration::AppConfiguration, + options: Reek::CLI::Options) + end + end end context 'when no source files given and no input was piped' do From aa2bea5b8cb8f64f4d88c8e0ccfc4573259f0e11 Mon Sep 17 00:00:00 2001 From: Paul Sadauskas Date: Sun, 24 Jun 2018 20:14:21 -0600 Subject: [PATCH 2/2] Change Examiner to always expect a SourceCode Also allow `SourceCode.from` to noop if the provided source is already a `SourceCode` --- features/command_line_interface/stdin.feature | 1 + lib/reek/cli/command/report_command.rb | 4 ++-- lib/reek/examiner.rb | 15 +++++++-------- lib/reek/source/source_code.rb | 8 ++++---- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/features/command_line_interface/stdin.feature b/features/command_line_interface/stdin.feature index 18359ce67..6f91d38de 100644 --- a/features/command_line_interface/stdin.feature +++ b/features/command_line_interface/stdin.feature @@ -67,6 +67,7 @@ Feature: Reek reads from $stdin when no files are given end """ Then it succeeds + And it reports nothing diff --git a/lib/reek/cli/command/report_command.rb b/lib/reek/cli/command/report_command.rb index 2fdd529b0..7a0c0ad48 100644 --- a/lib/reek/cli/command/report_command.rb +++ b/lib/reek/cli/command/report_command.rb @@ -23,8 +23,8 @@ def execute def populate_reporter_with_smells sources.each do |source| - reporter.add_examiner Examiner.new(source, - stdin_filename: options.stdin_filename, + reporter.add_examiner Examiner.new(Source::SourceCode.from(source), + origin: options.stdin_filename, filter_by_smells: smell_names, configuration: configuration, error_handler: LoggingErrorHandler.new) diff --git a/lib/reek/examiner.rb b/lib/reek/examiner.rb index ee6f9c190..ffacd4124 100644 --- a/lib/reek/examiner.rb +++ b/lib/reek/examiner.rb @@ -11,6 +11,7 @@ module Reek # Applies all available smell detectors to a source. # # @public + # :reek:TooManyInstanceVariables { max_instance_variables: 5 } class Examiner # Handles no errors class NullHandler @@ -33,14 +34,16 @@ def handle(_exception) # The configuration for this Examiner. # # @public + # :reek:ControlParameter # :reek:LongParameterList { max_params: 6 } def initialize(source, - stdin_filename: nil, + origin: nil, filter_by_smells: [], configuration: Configuration::AppConfiguration.default, detector_repository_class: DetectorRepository, error_handler: NullHandler.new) - @source = Source::SourceCode.from(source, filename: stdin_filename) + @source = Source::SourceCode.from(source) + @origin = origin || @source.origin @smell_types = detector_repository_class.eligible_smell_types(filter_by_smells) @detector_repository = detector_repository_class.new(smell_types: @smell_types, configuration: configuration.directive_for(description)) @@ -50,17 +53,13 @@ def initialize(source, # @return [String] origin of the source being analysed # # @public - def origin - @origin ||= source.origin - end + attr_reader :origin # @return [String] description of the source being analysed # # @public # @deprecated Use origin - def description - origin - end + alias description origin # # @return [Array] the smells found in the source diff --git a/lib/reek/source/source_code.rb b/lib/reek/source/source_code.rb index b0d0341b9..f4d388c8f 100644 --- a/lib/reek/source/source_code.rb +++ b/lib/reek/source/source_code.rb @@ -59,13 +59,13 @@ def initialize(code:, origin:, parser: self.class.default_parser) # # @return an instance of SourceCode # :reek:DuplicateMethodCall { max_calls: 2 } - # :reek:ControlParameter - def self.from(source, filename: nil) + def self.from(source) case source + when self then source when File then new(code: source.read, origin: source.path) - when IO then new(code: source.readlines.join, origin: filename || IO_IDENTIFIER) + when IO then new(code: source.readlines.join, origin: IO_IDENTIFIER) when Pathname then new(code: source.read, origin: source.to_s) - when String then new(code: source, origin: filename || STRING_IDENTIFIER) + when String then new(code: source, origin: STRING_IDENTIFIER) end end