diff --git a/inquisition.gemspec b/inquisition.gemspec index a4f867ab..beda1b34 100644 --- a/inquisition.gemspec +++ b/inquisition.gemspec @@ -28,7 +28,7 @@ Gem::Specification.new do |spec| spec.add_development_dependency 'simplecov', '~> 0.17.0' spec.add_development_dependency 'timecop', '~> 0.9.1' - spec.add_dependency 'active_record_doctor', '~> 1.6.0' + spec.add_dependency 'active_record_doctor', '~> 1.7', '>= 1.7.1' spec.add_dependency 'brakeman', '~> 4.6' spec.add_dependency 'bundler-audit', '~> 0.6' spec.add_dependency 'bundler-leak', '~> 0.1' diff --git a/lib/inquisition.rb b/lib/inquisition.rb index 5074a097..c14eba55 100644 --- a/lib/inquisition.rb +++ b/lib/inquisition.rb @@ -11,6 +11,14 @@ require 'inquisition/active_record_doctor/runner' require 'inquisition/active_record_doctor/issue' +require_relative 'inquisition/active_record_doctor/unindexed_foreign_keys_runner' +require_relative 'inquisition/active_record_doctor/extraneous_indexes_runner' +require_relative 'inquisition/active_record_doctor/missing_foreign_keys_runner' +require_relative 'inquisition/active_record_doctor/missing_non_null_constraint_runner' +require_relative 'inquisition/active_record_doctor/missing_presence_validation_runner' +require_relative 'inquisition/active_record_doctor/missing_unique_indexes_runner' +require_relative 'inquisition/active_record_doctor/undefined_table_references_runner' +require_relative 'inquisition/active_record_doctor/unindexed_deleted_at_runner' require 'inquisition/brakeman/vulnerability' require 'inquisition/brakeman/runner' require 'inquisition/rubocop' diff --git a/lib/inquisition/active_record_doctor/extraneous_indexes_runner.rb b/lib/inquisition/active_record_doctor/extraneous_indexes_runner.rb new file mode 100644 index 00000000..57c0c636 --- /dev/null +++ b/lib/inquisition/active_record_doctor/extraneous_indexes_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/extraneous_indexes' + +module Inquisition + module ActiveRecordDoctor + class ExtraneousIndexesRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::ExtraneousIndexes + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/active_record_doctor/issue.rb b/lib/inquisition/active_record_doctor/issue.rb index 23a3272b..194090e1 100644 --- a/lib/inquisition/active_record_doctor/issue.rb +++ b/lib/inquisition/active_record_doctor/issue.rb @@ -11,7 +11,7 @@ def to_h { severity: Severity::LOW, message: create_message, - context: task.to_s.split('::').last + context: task.demodulize } end @@ -20,7 +20,7 @@ def to_h attr_reader :task, :table, :column def create_message - issue_text = task.to_s.split('::').last.split(/(?=[A-Z])/).map(&:downcase).join(' ') + issue_text = task.demodulize.gsub(/([a-z]+)([A-Z])/, '\1 \2').downcase "#{table} has #{issue_text}, details: #{column ? column.join(', ') : 'n/a'}" end end diff --git a/lib/inquisition/active_record_doctor/missing_foreign_keys_runner.rb b/lib/inquisition/active_record_doctor/missing_foreign_keys_runner.rb new file mode 100644 index 00000000..5e43f939 --- /dev/null +++ b/lib/inquisition/active_record_doctor/missing_foreign_keys_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/missing_foreign_keys' + +module Inquisition + module ActiveRecordDoctor + class MissingForeignKeysRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::MissingForeignKeys + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/active_record_doctor/missing_non_null_constraint_runner.rb b/lib/inquisition/active_record_doctor/missing_non_null_constraint_runner.rb new file mode 100644 index 00000000..1565ea39 --- /dev/null +++ b/lib/inquisition/active_record_doctor/missing_non_null_constraint_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/missing_non_null_constraint' + +module Inquisition + module ActiveRecordDoctor + class MissingNonNullConstraintRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::MissingNonNullConstraint + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/active_record_doctor/missing_presence_validation_runner.rb b/lib/inquisition/active_record_doctor/missing_presence_validation_runner.rb new file mode 100644 index 00000000..7a33f694 --- /dev/null +++ b/lib/inquisition/active_record_doctor/missing_presence_validation_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/missing_presence_validation' + +module Inquisition + module ActiveRecordDoctor + class MissingPresenceValidationRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::MissingPresenceValidation + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/active_record_doctor/missing_unique_indexes_runner.rb b/lib/inquisition/active_record_doctor/missing_unique_indexes_runner.rb new file mode 100644 index 00000000..8c077747 --- /dev/null +++ b/lib/inquisition/active_record_doctor/missing_unique_indexes_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/missing_unique_indexes' + +module Inquisition + module ActiveRecordDoctor + class MissingUniqueIndexesRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::MissingUniqueIndexes + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/active_record_doctor/runner.rb b/lib/inquisition/active_record_doctor/runner.rb index c5552007..1f69813f 100644 --- a/lib/inquisition/active_record_doctor/runner.rb +++ b/lib/inquisition/active_record_doctor/runner.rb @@ -1,31 +1,22 @@ -require 'active_record_doctor/tasks/extraneous_indexes' -require 'active_record_doctor/tasks/missing_foreign_keys' -require 'active_record_doctor/tasks/unindexed_deleted_at' -require 'active_record_doctor/tasks/unindexed_foreign_keys' -require 'active_record_doctor/tasks/undefined_table_references' -require 'active_record_doctor/tasks/missing_unique_indexes' -require 'active_record_doctor/tasks/missing_presence_validation' -require 'active_record_doctor/tasks/missing_non_null_constraint' - module Inquisition module ActiveRecordDoctor class Runner < ::Inquisition::Runner - TASKS = [ - ::ActiveRecordDoctor::Tasks::ExtraneousIndexes, - ::ActiveRecordDoctor::Tasks::MissingForeignKeys, - ::ActiveRecordDoctor::Tasks::MissingNonNullConstraint, - ::ActiveRecordDoctor::Tasks::MissingPresenceValidation, - ::ActiveRecordDoctor::Tasks::MissingUniqueIndexes, - ::ActiveRecordDoctor::Tasks::UndefinedTableReferences, - ::ActiveRecordDoctor::Tasks::UnindexedDeletedAt, - ::ActiveRecordDoctor::Tasks::UnindexedForeignKeys - ].freeze + RUNNERS = %w[ExtraneousIndexesRunner + MissingForeignKeysRunner + MissingNonNullConstraintRunner + MissingPresenceValidationRunner + MissingUniqueIndexesRunner + UndefinedTableReferencesRunner + UnindexedDeletedAtRunner + UnindexedForeignKeysRunner].freeze def call - TASKS.each do |ard_task| - ard_task.run.first.each do |table, column| - @issues << Inquisition::Issue.new(Issue.new(ard_task, table, column).to_h.merge(runner: self)) - end + RUNNERS.each do |runner| + runner = "Inquisition::ActiveRecordDoctor::#{runner}".constantize.new + runner.run + issues = runner.issues + + issues.each { |issue| @issues << Inquisition::Issue.new(issue) } end @issues end diff --git a/lib/inquisition/active_record_doctor/undefined_table_references_runner.rb b/lib/inquisition/active_record_doctor/undefined_table_references_runner.rb new file mode 100644 index 00000000..27d7b45d --- /dev/null +++ b/lib/inquisition/active_record_doctor/undefined_table_references_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/undefined_table_references' + +module Inquisition + module ActiveRecordDoctor + class UndefinedTableReferencesRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::UndefinedTableReferences + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/active_record_doctor/unindexed_deleted_at_runner.rb b/lib/inquisition/active_record_doctor/unindexed_deleted_at_runner.rb new file mode 100644 index 00000000..3f72d5b0 --- /dev/null +++ b/lib/inquisition/active_record_doctor/unindexed_deleted_at_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/unindexed_deleted_at' + +module Inquisition + module ActiveRecordDoctor + class UnindexedDeletedAtRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::UnindexedDeletedAt + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/active_record_doctor/unindexed_foreign_keys_runner.rb b/lib/inquisition/active_record_doctor/unindexed_foreign_keys_runner.rb new file mode 100644 index 00000000..9ac00362 --- /dev/null +++ b/lib/inquisition/active_record_doctor/unindexed_foreign_keys_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/unindexed_foreign_keys' + +module Inquisition + module ActiveRecordDoctor + class UnindexedForeignKeysRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::UnindexedForeignKeys + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/outputter/doc/tpl/quality.rb b/lib/inquisition/outputter/doc/tpl/quality.rb index 646a43ed..f59e8afc 100644 --- a/lib/inquisition/outputter/doc/tpl/quality.rb +++ b/lib/inquisition/outputter/doc/tpl/quality.rb @@ -18,6 +18,12 @@ def rubocop Template.new('quality/rubocop').render(Rubocop.call(@issues)) end end + + def rails_best_practices + @rails_best_practices ||= begin + Template.new('quality/rails_best_practices').render(RailsBestPractices.call(@issues)) + end + end end end end @@ -26,3 +32,4 @@ def rubocop require_relative 'quality/rubycritic' require_relative 'quality/rubocop' +require_relative 'quality/rails_best_practices' diff --git a/lib/inquisition/outputter/doc/tpl/quality/rails_best_practices.rb b/lib/inquisition/outputter/doc/tpl/quality/rails_best_practices.rb new file mode 100644 index 00000000..ac3ebb5c --- /dev/null +++ b/lib/inquisition/outputter/doc/tpl/quality/rails_best_practices.rb @@ -0,0 +1,45 @@ +module Inquisition + module Outputter + class Doc + module TPL + class Quality + class RailsBestPractices + class Wrapper < SimpleDelegator + def group + group_by(&:severity).each do |severity, collection| + yield(severity.name.to_s.capitalize, collection) + end + end + end + + def self.call(issues) + new( + Wrapper.new(Security::Collector.new(issues, ::Inquisition::RailsBestPractices::Runner).call) + ) + end + + def produce + binding + end + + def types(issues) + issues.group_by(&:message).each do |warning, collection| + yield(warning, collection.count) + end + end + + attr_reader :collection + + def initialize(collection) + @collection = collection + end + + def link + @link ||= Stack::Collector.new(['rails_best_practices']).call.first.homepage + end + end + end + end + end + end +end diff --git a/spec/fixtures/files/active_record_doctor.yml b/spec/fixtures/files/active_record_doctor.yml index 0cbc063b..150ead79 100644 --- a/spec/fixtures/files/active_record_doctor.yml +++ b/spec/fixtures/files/active_record_doctor.yml @@ -3,7 +3,9 @@ - severity: :low message: 'projects has missing foreign keys, details: user_id' - severity: :low - message: 'ActiveStorage::Attachment has missing presence validation, details: name, record_type, record_id, blob_id' + message: 'projects has missing non null constraint, details: name, user_id' +- severity: :low + message: 'ActiveStorage::Attachment has missing presence validation, details: name, record_type' - severity: :low message: 'ActiveStorage::Blob has missing presence validation, details: key, filename, byte_size, checksum' - severity: :low diff --git a/spec/inquisition/active_record_doctor/issue_spec.rb b/spec/inquisition/active_record_doctor/issue_spec.rb index 540099f8..b006f78e 100644 --- a/spec/inquisition/active_record_doctor/issue_spec.rb +++ b/spec/inquisition/active_record_doctor/issue_spec.rb @@ -2,7 +2,7 @@ describe '#to_h' do subject(:vulnerability) { described_class.new(task, 'table', ['column#1', 'column#2']) } - let(:task) { ActiveRecordDoctor::Tasks::UnindexedForeignKeys } + let(:task) { ActiveRecordDoctor::Tasks::UnindexedForeignKeys.name } let(:message) { 'table has unindexed foreign keys, details: column#1, column#2' } let(:options) do diff --git a/spec/inquisition/active_record_doctor/runner_spec.rb b/spec/inquisition/active_record_doctor/runner_spec.rb index e7d2704e..bdbb0923 100644 --- a/spec/inquisition/active_record_doctor/runner_spec.rb +++ b/spec/inquisition/active_record_doctor/runner_spec.rb @@ -3,6 +3,7 @@ describe '#call' do let(:runner) { described_class.new } + let(:ard_runner) { 'UnindexedForeignKeysRunner' } let(:ard_task) { ActiveRecordDoctor::Tasks::UnindexedForeignKeys } let(:warning) { { 'unindexed_table' => %w[unindexed_column_1 unindexed_column_1] } } let(:message) do @@ -12,7 +13,7 @@ before { allow(ard_task).to receive(:run).and_return([warning, true]) } it 'returns issues array with specific message' do - stub_const('Inquisition::ActiveRecordDoctor::Runner::TASKS', [ard_task]) + stub_const('Inquisition::ActiveRecordDoctor::Runner::RUNNERS', [ard_runner]) expect(runner.call.first.message).to eq(message) end end diff --git a/views/doc/quality.rhtml b/views/doc/quality.rhtml index c300c6ff..1d56ee39 100644 --- a/views/doc/quality.rhtml +++ b/views/doc/quality.rhtml @@ -12,41 +12,10 @@ <%= rubocop %> -
- Rails code quality checking - +
  • + +
    • diff --git a/views/doc/quality/rails_best_practices.rhtml b/views/doc/quality/rails_best_practices.rhtml new file mode 100644 index 00000000..d02b9c7c --- /dev/null +++ b/views/doc/quality/rails_best_practices.rhtml @@ -0,0 +1,34 @@ +
    + Rails code quality checking + + + The full report can be found here. + +