diff --git a/spec/code_keeper/rubocop_differential_spec.rb b/spec/code_keeper/rubocop_differential_spec.rb new file mode 100644 index 0000000..5fbb41b --- /dev/null +++ b/spec/code_keeper/rubocop_differential_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "spec_helper" +require "support/corpus_report_runner" +require "support/rubocop_metric_oracle" +require "support/rubocop_metric_differential" + +RSpec.describe "RuboCop metric differential" do + it "keeps wrapper cops out of the global RuboCop registry" do + expected_cops = { + "Metrics/AbcSize" => RuboCop::Cop::Metrics::AbcSize, + "Metrics/CyclomaticComplexity" => RuboCop::Cop::Metrics::CyclomaticComplexity, + "Metrics/ClassLength" => RuboCop::Cop::Metrics::ClassLength, + "Metrics/ModuleLength" => RuboCop::Cop::Metrics::ModuleLength + } + registered_cops = expected_cops.keys.to_h do |cop_name| + [cop_name, RuboCop::Cop::Registry.global.find_by_cop_name(cop_name)] + end + + expect(registered_cops).to eq expected_cops + end + + it "keeps accepted-difference baseline entries well-formed" do + expect(RuboCopMetricDifferential.validate_baseline_entries).to eq [] + end + + it "matches RuboCop metric values over the vendored corpus" do + result = RuboCopMetricDifferential.compare(CorpusReportRunner.file_paths) + + aggregate_failures do + expect(result.duplicate_keys.map(&:to_h)).to eq [] + expect(result.value_mismatches.map(&:to_h)).to eq [] + expect(result.unexpected_differences.map(&:to_h)).to eq [] + expect(result.stale_baseline_entries).to eq [] + end + end +end diff --git a/spec/fixtures/rubocop_differential_baseline.yml b/spec/fixtures/rubocop_differential_baseline.yml new file mode 100644 index 0000000..ab7b921 --- /dev/null +++ b/spec/fixtures/rubocop_differential_baseline.yml @@ -0,0 +1,22 @@ +--- +accepted_differences: + - metric: class_length + path: spec/fixtures/corpus/gitlab/app/models/project.rb + line: 1243 + side: code_keeper + reason: CodeKeeper measures class-body class << self as a statically resolved singleton class; RuboCop Metrics/ClassLength skips singleton classes inside class bodies. + - metric: class_length + path: spec/fixtures/corpus/gitlab/app/models/merge_request.rb + line: 934 + side: code_keeper + reason: CodeKeeper measures class-body class << self as a statically resolved singleton class; RuboCop Metrics/ClassLength skips singleton classes inside class bodies. + - metric: class_length + path: spec/fixtures/corpus/gitlab/app/models/ability.rb + line: 4 + side: code_keeper + reason: CodeKeeper measures class-body class << self as a statically resolved singleton class; RuboCop Metrics/ClassLength skips singleton classes inside class bodies. + - metric: class_length + path: spec/fixtures/corpus/gitlab/app/models/concerns/issuable.rb + line: 154 + side: rubocop + reason: RuboCop Metrics/ClassLength measures class << self inside the included do DSL block, while CodeKeeper skips dynamic self singleton classes. diff --git a/spec/support/rubocop_metric_differential.rb b/spec/support/rubocop_metric_differential.rb new file mode 100644 index 0000000..4c89f0f --- /dev/null +++ b/spec/support/rubocop_metric_differential.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +require "support/rubocop_metric_differential_types" +require "support/rubocop_metric_differential_baseline" + +# Compares CodeKeeper measurements with raw values captured from RuboCop's cop +# pipeline and applies an explicit accepted-differences baseline. +module RuboCopMetricDifferential + BASELINE_PATH = "spec/fixtures/rubocop_differential_baseline.yml" + + module_function + + def compare(paths) + state = ComparisonState.new(differences: [], duplicate_keys: [], value_mismatches: []) + + paths.each do |path| + code_keeper_measurements = code_keeper_measurements_for(path) + rubocop_measurements = RuboCopMetricOracle.measurements_for(path) + compare_measurements(code_keeper_measurements, rubocop_measurements, state) + end + + Result.new( + unexpected_differences: unexpected_differences(state.differences), + stale_baseline_entries: stale_baseline_entries(state.differences), + duplicate_keys: state.duplicate_keys, + value_mismatches: state.value_mismatches + ) + end + + def validate_baseline_entries + Baseline.validate_entries + end + + def compare_measurements(code_keeper_measurements, rubocop_measurements, state) + code_keeper_groups = code_keeper_measurements.group_by { |measurement| measurement_key(measurement) } + rubocop_groups = rubocop_measurements.group_by { |measurement| measurement_key(measurement) } + + (code_keeper_groups.keys | rubocop_groups.keys).each do |key| + code_keeper_group = code_keeper_groups.fetch(key, []) + rubocop_group = rubocop_groups.fetch(key, []) + compare_group(key, code_keeper_group, rubocop_group, state) + end + end + + def compare_group(key, code_keeper_group, rubocop_group, state) + record_duplicate_key(:code_keeper, key, code_keeper_group, state) + record_duplicate_key(:rubocop, key, rubocop_group, state) + + if code_keeper_group.empty? + rubocop_group.each { |measurement| state.differences << difference_from(:rubocop, measurement) } + elsif rubocop_group.empty? + code_keeper_group.each { |measurement| state.differences << difference_from(:code_keeper, measurement) } + elsif code_keeper_group.one? && rubocop_group.one? + compare_value(code_keeper_group.first, rubocop_group.first, state) + end + end + + def compare_value(code_keeper_measurement, rubocop_measurement, state) + return if code_keeper_measurement.value == rubocop_measurement.value + + state.value_mismatches << ValueMismatch.new( + metric: code_keeper_measurement.metric, + path: code_keeper_measurement.path, + line: code_keeper_measurement.start_line, + code_keeper_value: code_keeper_measurement.value, + rubocop_value: rubocop_measurement.value + ) + end + + def record_duplicate_key(side, key, measurements, state) + return unless measurements.size > 1 + + metric, path, line = key + state.duplicate_keys << DuplicateKey.new( + metric: metric, + path: path, + line: line, + side: side, + measurement_values: measurements.map(&:value) + ) + end + + def difference_from(side, measurement) + Difference.new( + metric: measurement.metric, + path: measurement.path, + line: measurement.start_line, + side: side, + value: measurement.value + ) + end + + def unexpected_differences(differences) + accepted = Baseline.identities + + differences.reject { |difference| accepted.include?(difference.identity) } + end + + def stale_baseline_entries(differences) + observed = differences.map(&:identity) + + Baseline.entries.reject { |entry| observed.include?(Baseline.identity(entry)) } + end + + def code_keeper_measurements_for(path) + CorpusReportRunner.report_for(path, number_of_threads: 1).measurements + end + + def measurement_key(measurement) + [measurement.metric, measurement.path, measurement.start_line] + end +end diff --git a/spec/support/rubocop_metric_differential_baseline.rb b/spec/support/rubocop_metric_differential_baseline.rb new file mode 100644 index 0000000..96ce990 --- /dev/null +++ b/spec/support/rubocop_metric_differential_baseline.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require "yaml" + +module RuboCopMetricDifferential + # Loads and validates accepted differences for the RuboCop differential. + module Baseline + REQUIRED_KEYS = %w[metric path line side reason].freeze + SIDES = %w[code_keeper rubocop].freeze + + module_function + + def entries + YAML.safe_load_file(BASELINE_PATH).fetch("accepted_differences") + end + + def identities + entries.map { |entry| identity(entry) } + end + + def validate_entries + entries.filter_map do |entry| + missing = missing_keys(entry) + invalid = entry_errors(entry) + next if missing.empty? && invalid.empty? + + entry.merge("missing_keys" => missing, "invalid_values" => invalid) + end + end + + def identity(entry) + { + "metric" => entry.fetch("metric"), + "path" => entry.fetch("path"), + "line" => entry.fetch("line"), + "side" => entry.fetch("side") + } + end + + def missing_keys(entry) + REQUIRED_KEYS.reject { |key| entry.key?(key) } + end + + def entry_errors(entry) + [ + metric_error(entry), + path_error(entry), + line_error(entry), + side_error(entry), + reason_error(entry) + ].compact + end + + def metric_error(entry) + "metric" unless CodeKeeper::Metrics::MAPPINGS.key?(entry["metric"]&.to_sym) + end + + def path_error(entry) + "path" unless entry["path"].is_a?(String) && entry["path"].start_with?(CorpusReportRunner::ROOT) + end + + def line_error(entry) + "line" unless entry["line"].is_a?(Integer) && entry["line"].positive? + end + + def side_error(entry) + "side" unless SIDES.include?(entry["side"]) + end + + def reason_error(entry) + "reason" unless entry["reason"].is_a?(String) && !entry["reason"].empty? + end + end +end diff --git a/spec/support/rubocop_metric_differential_types.rb b/spec/support/rubocop_metric_differential_types.rb new file mode 100644 index 0000000..ba10572 --- /dev/null +++ b/spec/support/rubocop_metric_differential_types.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module RuboCopMetricDifferential + # A measurement present on only one side of the differential. + Difference = Struct.new(:metric, :path, :line, :side, :value, keyword_init: true) do + def identity + { + "metric" => metric.to_s, + "path" => path, + "line" => line, + "side" => side.to_s + } + end + + def to_h + identity.merge("value" => value) + end + end + + # A non-unique path/line key on one side of the differential. + DuplicateKey = Struct.new(:metric, :path, :line, :side, :measurement_values, keyword_init: true) do + def to_h + { + "metric" => metric.to_s, + "path" => path, + "line" => line, + "side" => side.to_s, + "values" => measurement_values + } + end + end + + # A measurement key present on both sides with different metric values. + ValueMismatch = Struct.new(:metric, :path, :line, :code_keeper_value, :rubocop_value, keyword_init: true) do + def to_h + { + "metric" => metric.to_s, + "path" => path, + "line" => line, + "code_keeper_value" => code_keeper_value, + "rubocop_value" => rubocop_value + } + end + end + + # Mutable state used while comparing one corpus run. + ComparisonState = Struct.new(:differences, :duplicate_keys, :value_mismatches, keyword_init: true) + + # Final differential result consumed by specs. + Result = Struct.new(:unexpected_differences, :stale_baseline_entries, :duplicate_keys, :value_mismatches, keyword_init: true) +end diff --git a/spec/support/rubocop_metric_oracle.rb b/spec/support/rubocop_metric_oracle.rb new file mode 100644 index 0000000..9e988ca --- /dev/null +++ b/spec/support/rubocop_metric_oracle.rb @@ -0,0 +1,206 @@ +# frozen_string_literal: true + +# Runs RuboCop metric cops through the normal cop pipeline and captures raw +# metric values before offense messages round them for display. +module RuboCopMetricOracle + TARGET_RUBY_VERSION = RUBY_VERSION.split(".").first(2).join(".").to_f + + Measurement = Struct.new(:metric, :path, :start_line, :end_line, :value, keyword_init: true) do + def key + [metric, path, start_line] + end + + def to_h + { + metric: metric, + path: path, + start_line: start_line, + end_line: end_line, + value: value + } + end + end + + # Keeps wrapper cops tied to the original RuboCop cop config and badge. + module CopIdentity + def cop_name + rubocop_cop_name + end + + def badge + RuboCop::Cop::Badge.parse(rubocop_cop_name) + end + end + + # Stores raw metric measurements observed during one RuboCop investigation. + module CapturesMeasurements + attr_reader :measurements + + def on_new_investigation + super + @measurements = [] + end + + private + + def capture_measurement(metric, node, value) + measurements << Measurement.new( + metric: metric, + path: processed_source.file_path, + start_line: node.first_line, + end_line: node.last_line, + value: value + ) + end + end + + # Captures ABC values before RuboCop formats an offense message. + class CapturedAbcSize < RuboCop::Cop::Metrics::AbcSize + extend CopIdentity + include CapturesMeasurements + exclude_from_registry + + def self.rubocop_cop_name + "Metrics/AbcSize" + end + + private + + def check_complexity(node, method_name) + unless node.body + capture_measurement(:abc_metric, node, 0) + return + end + + reset_repeated_csend + value, = complexity(node.body) + capture_measurement(:abc_metric, node, value) + super + end + end + + # Captures cyclomatic complexity values before offense reporting. + class CapturedCyclomaticComplexity < RuboCop::Cop::Metrics::CyclomaticComplexity + extend CopIdentity + include CapturesMeasurements + exclude_from_registry + + def self.rubocop_cop_name + "Metrics/CyclomaticComplexity" + end + + private + + def check_complexity(node, method_name) + unless node.body + capture_measurement(:cyclomatic_complexity, node, 1) + return + end + + reset_repeated_csend + capture_measurement(:cyclomatic_complexity, node, complexity(node.body)) + super + end + end + + # Captures class length values before offense reporting. + class CapturedClassLength < RuboCop::Cop::Metrics::ClassLength + extend CopIdentity + include CapturesMeasurements + exclude_from_registry + + def self.rubocop_cop_name + "Metrics/ClassLength" + end + + private + + def check_code_length(node) + capture_measurement(:class_length, node, build_code_length_calculator(node).calculate) + super + end + end + + # CodeKeeper's class_length metric covers modules too, so the RuboCop oracle + # needs ModuleLength in addition to ClassLength for that shared measurement. + class CapturedModuleLength < RuboCop::Cop::Metrics::ModuleLength + extend CopIdentity + include CapturesMeasurements + exclude_from_registry + + def self.rubocop_cop_name + "Metrics/ModuleLength" + end + + private + + def check_code_length(node) + capture_measurement(:class_length, node, build_code_length_calculator(node).calculate) + super + end + end + + COP_CLASSES = [ + CapturedAbcSize, + CapturedCyclomaticComplexity, + CapturedClassLength, + CapturedModuleLength + ].freeze + + module_function + + def measurements_for(path) + processed_source = RuboCop::AST::ProcessedSource.new(File.read(path), TARGET_RUBY_VERSION, path) + team = RuboCop::Cop::Team.mobilize(COP_CLASSES, config, team_options) + report = team.investigate(processed_source) + raise report.errors.map(&:message).join("\n") unless report.errors.empty? + + team.cops.flat_map(&:measurements) + end + + def config + @config ||= RuboCop::Config.new( + { + "AllCops" => { + "NewCops" => "disable", + "TargetRubyVersion" => TARGET_RUBY_VERSION + }, + "Metrics/AbcSize" => { + "Enabled" => true, + "Max" => 0, + "CountRepeatedAttributes" => true, + "AllowedMethods" => [], + "AllowedPatterns" => [] + }, + "Metrics/CyclomaticComplexity" => { + "Enabled" => true, + "Max" => 0, + "AllowedMethods" => [], + "AllowedPatterns" => [] + }, + "Metrics/ClassLength" => { + "Enabled" => true, + "Max" => 0, + "CountComments" => false, + "CountAsOne" => [] + }, + "Metrics/ModuleLength" => { + "Enabled" => true, + "Max" => 0, + "CountComments" => false, + "CountAsOne" => [] + } + }, + nil + ) + end + + def team_options + { + autocorrect: false, + debug: false, + ignore_disable_comments: true, + raise_error: true + } + end +end