diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 9a537186dbd..b01d005cc19 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -27,6 +27,7 @@ jobs: uses: actions/setup-python@v3 with: python-version: "3.10" + - name: Install dependencies run: | python -m pip install --upgrade pip @@ -34,19 +35,8 @@ jobs: - name: Check spelling with codespell run: codespell --ignore-words=codespell.txt --skip="./vendor/bundle,./actionview/test/ujs/public/vendor/qunit.js,./actiontext/app/assets/javascripts/trix.js,./yarn.lock" || exit 1 - - uses: actions/checkout@v4 - with: - repository: skipkayhil/rails-bin - ref: 748f4673a5fe5686b5859e89f814166280e51781 - - uses: ruby/setup-ruby@v1 - with: - ruby-version: 3.2 - bundler-cache: true - - uses: actions/checkout@v4 - with: - path: rails - - run: bin/check-changelogs ./rails - - run: bin/check-config-docs ./rails + - run: tools/railspect changelogs . + - run: tools/railspect configuration . - uses: zzak/action-discord@v8 continue-on-error: true diff --git a/.github/workflows/rail_inspector.yml b/.github/workflows/rail_inspector.yml new file mode 100644 index 00000000000..ac7dd091faf --- /dev/null +++ b/.github/workflows/rail_inspector.yml @@ -0,0 +1,24 @@ +name: Rail Inspector + +on: + pull_request: + paths: + - "tools/rail_inspector/**" + push: + paths: + - "tools/rail_inspector/**" + +permissions: + contents: read + +jobs: + rail_inspector: + name: rail_inspector tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: ruby/setup-ruby@v1 + with: + ruby-version: 3.2 + bundler-cache: true + - run: cd tools/rail_inspector && bundle exec rake diff --git a/.rubocop.yml b/.rubocop.yml index 91fff120d77..08c8e60fe10 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -19,6 +19,7 @@ AllCops: - 'actionmailbox/test/dummy/**/*' - 'activestorage/test/dummy/**/*' - 'actiontext/test/dummy/**/*' + - 'tools/rail_inspector/test/fixtures/*' - '**/node_modules/**/*' - '**/CHANGELOG.md' - '**/2_*_release_notes.md' diff --git a/Gemfile b/Gemfile index d9de12e1e38..b208b3e9b18 100644 --- a/Gemfile +++ b/Gemfile @@ -45,6 +45,10 @@ gem "json", ">= 2.0.0" # Workaround until Ruby ships with cgi version 0.3.6 or higher. gem "cgi", ">= 0.3.6", require: false +group :lint do + gem "syntax_tree", "6.1.1", require: false +end + group :rubocop do gem "rubocop", ">= 1.25.1", require: false gem "rubocop-minitest", require: false diff --git a/Gemfile.lock b/Gemfile.lock index 83adc65d13e..ef4e7a5f61d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -369,6 +369,7 @@ GEM ast (~> 2.4.1) path_expander (1.1.1) pg (1.5.3) + prettier_print (1.2.1) propshaft (0.6.4) actionpack (>= 7.0.0) activesupport (>= 7.0.0) @@ -513,6 +514,8 @@ GEM stringio (3.0.7) sucker_punch (3.1.0) concurrent-ruby (~> 1.0) + syntax_tree (6.1.1) + prettier_print (>= 1.2.0) tailwindcss-rails (2.0.21) railties (>= 6.0.0) tailwindcss-rails (2.0.21-x86_64-darwin) @@ -631,6 +634,7 @@ DEPENDENCIES stackprof stimulus-rails sucker_punch + syntax_tree (= 6.1.1) tailwindcss-rails terser (>= 1.1.4) trilogy (>= 2.5.0) diff --git a/tools/README.md b/tools/README.md index f133b27128d..2dbe65d14a3 100644 --- a/tools/README.md +++ b/tools/README.md @@ -5,6 +5,7 @@ They aren't used by Rails apps directly. * `console` drops you in irb and loads local Rails repos * `profile` profiles `Kernel#require` to help reduce startup time + * `railspect` provides commands to run internal linters * `line_statistics` provides CodeTools module and LineStatistics class to count lines * `test` is loaded by every major component of Rails to simplify testing, for example: `cd ./actioncable; bin/test ./path/to/actioncable_test_with_line_number.rb:5` diff --git a/tools/rail_inspector/LICENSE b/tools/rail_inspector/LICENSE new file mode 100644 index 00000000000..d39c8b0f421 --- /dev/null +++ b/tools/rail_inspector/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) Hartley McGuire + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/tools/rail_inspector/Rakefile b/tools/rail_inspector/Rakefile new file mode 100644 index 00000000000..478656f61ed --- /dev/null +++ b/tools/rail_inspector/Rakefile @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require "bundler/gem_tasks" +require "minitest/test_task" +require "rubocop/rake_task" + +Minitest::TestTask.create + +RuboCop::RakeTask.new + +task default: [:test, :rubocop] diff --git a/tools/rail_inspector/bin/console b/tools/rail_inspector/bin/console new file mode 100755 index 00000000000..534eaa8d0b5 --- /dev/null +++ b/tools/rail_inspector/bin/console @@ -0,0 +1,15 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +require "bundler/setup" +require_relative "../lib/rail_inspector" + +# You can add fixtures and/or initialization code here to make experimenting +# with your gem easier. You can also use a different console, if you like. + +# (If you use this, don't forget to add pry to your Gemfile!) +# require "pry" +# Pry.start + +require "irb" +IRB.start(__FILE__) diff --git a/tools/rail_inspector/lib/rail_inspector.rb b/tools/rail_inspector/lib/rail_inspector.rb new file mode 100644 index 00000000000..98dd03fe925 --- /dev/null +++ b/tools/rail_inspector/lib/rail_inspector.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require_relative "rail_inspector/version" + +# MIT License +# +# Copyright (c) Hartley McGuire +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + +module RailInspector +end diff --git a/tools/rail_inspector/lib/rail_inspector/changelog.rb b/tools/rail_inspector/lib/rail_inspector/changelog.rb new file mode 100644 index 00000000000..aace936efa7 --- /dev/null +++ b/tools/rail_inspector/lib/rail_inspector/changelog.rb @@ -0,0 +1,257 @@ +# frozen_string_literal: true + +require "pathname" +require "strscan" + +module RailInspector + class Changelog + class Offense + attr_reader :line, :line_number, :range, :message + + def initialize(line, line_number, range, message) + @line = line + @line_number = line_number + @range = range + @message = message + end + end + + class Entry + attr_reader :lines, :offenses + + def initialize(lines, starting_line) + @lines = lines + @starting_line = starting_line + + @offenses = [] + + validate_authors + validate_leading_whitespace + validate_trailing_whitespace + end + + private + def header + lines.first + end + + def validate_authors + authors = + lines.reverse.find { |line| line.match?(/\*[^\d\s]+(\s[^\d\s]+)*\*/) } + + return if authors + + add_offense( + header, + line_in_file(0), + 1..header.length, + "CHANGELOG entry is missing authors." + ) + end + + def validate_leading_whitespace + unless header.match?(/\* {3}\S/) + add_offense( + header, + line_in_file(0), + 1..4, + "CHANGELOG header must start with '*' and 3 spaces" + ) + end + + lines.each_with_index do |line, i| + next if i == 0 + next if line.strip.empty? + next if line.start_with?(" " * 4) + + add_offense( + line, + line_in_file(i), + 1..4, + "CHANGELOG line must be indented 4 spaces" + ) + end + end + + def validate_trailing_whitespace + lines.each_with_index do |line, i| + next unless line.end_with?(" ", "\t") + + add_offense( + line, + line_in_file(i), + (line.rstrip.length + 1)..line.length, + "Trailing whitespace detected." + ) + end + end + + private + def add_offense(...) + @offenses << Offense.new(...) + end + + def line_in_file(line_in_entry) + @starting_line + line_in_entry + end + end + + class Parser + def self.call(file) + new(file).parse + end + + def self.to_proc + method(:call).to_proc + end + + def initialize(file) + @buffer = StringScanner.new(file) + @lines = [] + @current_line = 1 + + @entries = [] + end + + def parse + until @buffer.eos? + if peek_footer? + pop_entry + next parse_footer + end + + pop_entry if peek_probably_header? + + parse_line + end + + @entries + end + + private + def parse_line + @current_line += 1 + @lines << @buffer.scan_until(/\n/)[0...-1] + end + + FOOTER_TEXT = "Please check" + + def parse_footer + @buffer.scan( + /#{FOOTER_TEXT} \[\d-\d-stable\]\(.*\) for previous changes\.\n/o + ) + end + + def peek_probably_header? + return false unless @buffer.peek(1) == "*" + + maybe_header = @buffer.check_until(/\n/).strip + + # If there are an odd number of *, then the line is almost certainly a + # header since bolding requires pairs. + return true unless maybe_header.count("*").even? + + !maybe_header.end_with?("*") + end + + def peek_footer? + @buffer.peek(FOOTER_TEXT.length) == FOOTER_TEXT + end + + def pop_entry + # Ensure we don't pop an entry if we only see newlines and the footer + return unless @lines.any? { |line| line.match?(/\S/) } + + @entries << Changelog::Entry.new(@lines, @current_line - @lines.length) + @lines = [] + end + end + + class Formatter + def initialize + @changelog_count = 0 + @offense_count = 0 + end + + def to_proc + method(:call).to_proc + end + + def call(changelog) + @changelog_count += 1 + + changelog.offenses.each { |o| process_offense(changelog, o) } + end + + def finish + puts "#{@changelog_count} changelogs inspected, #{@offense_count} offense#{"s" unless @offense_count == 1} detected" + end + + private + def process_offense(file, offense) + @offense_count += 1 + + puts "#{file.path}:#{offense.line_number} #{offense.message}" + puts offense.line + puts ("^" * offense.range.count).rjust(offense.range.end) + end + end + + class Runner + attr_reader :formatter, :rails_path + + def initialize(rails_path) + @formatter = Formatter.new + @rails_path = Pathname.new(rails_path) + end + + def call + invalid_changelogs = + changelogs.reject do |changelog| + output = changelog.valid? ? "." : "E" + $stdout.write(output) + + changelog.valid? + end + + puts "\n\n" + puts "Offenses:\n\n" unless invalid_changelogs.empty? + + changelogs.each(&formatter) + formatter.finish + + invalid_changelogs.empty? + end + + private + def changelogs + changelog_paths.map { |path| Changelog.new(path, File.read(path)) } + end + + def changelog_paths + Dir[rails_path.join("*/CHANGELOG.md")] + end + end + + attr_reader :path, :content, :entries + + def initialize(path, content) + @path = path + @content = content + @entries = parser.parse + end + + def valid? + offenses.empty? + end + + def offenses + @offenses ||= entries.flat_map(&:offenses) + end + + private + def parser + @parser ||= Parser.new(content) + end + end +end diff --git a/tools/rail_inspector/lib/rail_inspector/cli.rb b/tools/rail_inspector/lib/rail_inspector/cli.rb new file mode 100644 index 00000000000..1823f81a916 --- /dev/null +++ b/tools/rail_inspector/lib/rail_inspector/cli.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require "thor" + +module RailInspector + class Cli < Thor + class << self + def exit_on_failure? + true + end + end + + desc "changelogs RAILS_PATH", "Check CHANGELOG files for common issues" + def changelogs(rails_path) + require_relative "./changelog" + + exit Changelog::Runner.new(rails_path).call + end + + desc "configuration RAILS_PATH", "Check various Configuration issues" + option :autocorrect, type: :boolean, aliases: :a + def configuration(rails_path) + require_relative "./configuring" + + checker = Configuring.new(rails_path) + checker.check + + puts checker.errors unless checker.errors.empty? + exit checker.errors.empty? unless options[:autocorrect] + + checker.write! + end + end +end diff --git a/tools/rail_inspector/lib/rail_inspector/configuring.rb b/tools/rail_inspector/lib/rail_inspector/configuring.rb new file mode 100644 index 00000000000..debd0148490 --- /dev/null +++ b/tools/rail_inspector/lib/rail_inspector/configuring.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require "pathname" +require_relative "./configuring/check/general_configuration" +require_relative "./configuring/check/framework_defaults" + +module RailInspector + class Configuring + class CachedParser + def initialize + @cache = {} + end + + def call(path) + @cache[path] ||= SyntaxTree.parse(SyntaxTree.read(path)) + end + end + + DOC_PATH = "guides/source/configuring.md" + APPLICATION_CONFIGURATION_PATH = + "railties/lib/rails/application/configuration.rb" + NEW_FRAMEWORK_DEFAULTS_PATH = + "railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_%{version}.rb.tt" + + class Doc + attr_accessor :general_config, :versioned_defaults + + def initialize(content) + @before, @versioned_defaults, @general_config, @after = + content + .split("\n") + .slice_before do |line| + [ + "### Versioned Default Values", + "### Rails General Configuration", + "### Configuring Assets" + ].include?(line) + end + .to_a + end + + def to_s + (@before + @versioned_defaults + @general_config + @after).join("\n") + + "\n" + end + end + + attr_reader :errors, :parser + + def initialize(rails_path) + @errors = [] + @parser = CachedParser.new + @rails_path = Pathname.new(rails_path) + end + + def check + [Check::GeneralConfiguration, Check::FrameworkDefaults].each do |check| + check.new(self).check + end + end + + def doc + @doc ||= + begin + content = File.read(doc_path) + Configuring::Doc.new(content) + end + end + + def parse(relative_path) + parser.call(@rails_path.join(relative_path)) + end + + def read(relative_path) + File.read(@rails_path.join(relative_path)) + end + + def rails_version + @rails_version ||= File.read(@rails_path.join("RAILS_VERSION")).to_f.to_s + end + + def write! + File.write(doc_path, doc.to_s) + end + + private + def doc_path + @rails_path.join(DOC_PATH) + end + end +end diff --git a/tools/rail_inspector/lib/rail_inspector/configuring/check/framework_defaults.rb b/tools/rail_inspector/lib/rail_inspector/configuring/check/framework_defaults.rb new file mode 100644 index 00000000000..b41ea1fc26c --- /dev/null +++ b/tools/rail_inspector/lib/rail_inspector/configuring/check/framework_defaults.rb @@ -0,0 +1,134 @@ +# frozen_string_literal: true + +require "tempfile" +require_relative "../../visitor/framework_default" + +module RailInspector + class Configuring + module Check + class FrameworkDefaults + class NewFrameworkDefaultsFile + attr_reader :checker, :visitor + + def initialize(checker, visitor) + @checker = checker + @visitor = visitor + end + + def check + visitor.config_map[checker.rails_version].each_key do |config| + app_config = config.gsub(/^self/, "config") + + next if defaults_file_content.include? app_config + + add_error(config) + end + end + + private + def add_error(config) + checker.errors << <<~MESSAGE + #{new_framework_defaults_path} + Missing: #{config} + + MESSAGE + end + + def defaults_file_content + @defaults_file_content ||= checker.read(new_framework_defaults_path) + end + + def new_framework_defaults_path + NEW_FRAMEWORK_DEFAULTS_PATH % + { version: checker.rails_version.tr(".", "_") } + end + end + + attr_reader :checker + + def initialize(checker) + @checker = checker + end + + def check + header, *defaults_by_version = documented_defaults + + NewFrameworkDefaultsFile.new(checker, visitor).check + + checker.doc.versioned_defaults = + header + + defaults_by_version + .map { |defaults| check_defaults(defaults) } + .flatten + end + + private + def app_config_tree + checker.parse(APPLICATION_CONFIGURATION_PATH) + end + + def check_defaults(defaults) + header, configs = defaults[0], defaults[2, defaults.length - 3] + + version = header.match(/\d\.\d/)[0] + + generated_doc = + visitor.config_map[version] + .map do |config, value| + full_config = + case config + when /^[A-Z]/ + config + when /^self/ + config.sub("self", "config") + else + "config.#{config}" + end + + "- [`#{full_config}`](##{full_config.tr("._", "-").downcase}): `#{value}`" + end + .sort + + config_diff = + Tempfile.create("expected") do |doc| + doc << generated_doc.join("\n") + doc.flush + + Tempfile.create("actual") do |code| + code << configs.join("\n") + code.flush + + `git diff --color --no-index #{doc.path} #{code.path}` + end + end + + checker.errors << <<~MESSAGE unless config_diff.empty? + #{APPLICATION_CONFIGURATION_PATH}: Incorrect load_defaults docs + --- Expected + +++ Actual + #{config_diff.split("\n")[5..].join("\n")} + MESSAGE + + [header, "", *generated_doc, ""] + end + + def documented_defaults + checker + .doc + .versioned_defaults + .slice_before { |line| line.start_with?("####") } + .to_a + end + + def visitor + @visitor ||= + begin + visitor = Visitor::FrameworkDefault.new + visitor.visit(app_config_tree) + visitor + end + end + end + end + end +end diff --git a/tools/rail_inspector/lib/rail_inspector/configuring/check/general_configuration.rb b/tools/rail_inspector/lib/rail_inspector/configuring/check/general_configuration.rb new file mode 100644 index 00000000000..32d57eb85b3 --- /dev/null +++ b/tools/rail_inspector/lib/rail_inspector/configuring/check/general_configuration.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require_relative "../../visitor/attribute" + +module RailInspector + class Configuring + module Check + class GeneralConfiguration + attr_reader :checker + + def initialize(checker) + @checker = checker + end + + def check + header, *config_sections = documented_general_config + + non_nested_accessors = + general_accessors.reject do |a| + config_sections.any? { |section| /\.#{a}\./.match?(section[0]) } + end + + non_nested_accessors.each do |accessor| + config_header = "#### `config.#{accessor}`" + + unless config_sections.any? { |section| section[0] == config_header } + checker.errors << config_header + config_sections << [config_header, "", "FIXME", ""] + end + end + + checker.doc.general_config = + [header] + + config_sections.sort_by { |section| section[0].split("`")[1] } + end + + private + APP_CONFIG_CONST = "Rails::Application::Configuration" + + def app_config_tree + checker.parse(APPLICATION_CONFIGURATION_PATH) + end + + def documented_general_config + checker + .doc + .general_config + .slice_before { |line| line.start_with?("####") } + .to_a + end + + def general_accessors + visitor.attribute_map[APP_CONFIG_CONST]["attr_accessor"] + end + + def visitor + @visitor ||= + begin + visitor = Visitor::Attribute.new + visitor.visit(app_config_tree) + visitor + end + end + end + end + end +end diff --git a/tools/rail_inspector/lib/rail_inspector/version.rb b/tools/rail_inspector/lib/rail_inspector/version.rb new file mode 100644 index 00000000000..cfc482cf62a --- /dev/null +++ b/tools/rail_inspector/lib/rail_inspector/version.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +module RailInspector + VERSION = "0.0.2" +end diff --git a/tools/rail_inspector/lib/rail_inspector/visitor/attribute.rb b/tools/rail_inspector/lib/rail_inspector/visitor/attribute.rb new file mode 100644 index 00000000000..cc1166c741b --- /dev/null +++ b/tools/rail_inspector/lib/rail_inspector/visitor/attribute.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require "set" +require "syntax_tree" + +module RailInspector + module Visitor + class Attribute < SyntaxTree::Visitor + attr_reader :attribute_map + + def initialize + @attribute_map = {} + @namespace_stack = [] + end + + def with_namespace(node) + @namespace_stack << node.constant.constant.value + visit_child_nodes(node) + @namespace_stack.pop + end + + visit_method alias_method :visit_module, :with_namespace + + visit_method alias_method :visit_class, :with_namespace + + visit_method def visit_command(node) + attr_access = node.message.value + return unless ATTRIBUTE_METHODS.include?(attr_access) + + full_namespace = @namespace_stack.join("::") + + @attribute_map[full_namespace] ||= {} + @attribute_map[full_namespace][attr_access] ||= Set.new + + attributes = node.arguments.parts.map { |p| p.value.value } + + @attribute_map[full_namespace][attr_access].merge(attributes) + end + + private + ATTRIBUTE_METHODS = %w[attr_accessor attr_reader attr_writer] + end + end +end diff --git a/tools/rail_inspector/lib/rail_inspector/visitor/framework_default.rb b/tools/rail_inspector/lib/rail_inspector/visitor/framework_default.rb new file mode 100644 index 00000000000..7bf6c981035 --- /dev/null +++ b/tools/rail_inspector/lib/rail_inspector/visitor/framework_default.rb @@ -0,0 +1,132 @@ +# frozen_string_literal: true + +require "syntax_tree" + +require_relative "./hash_to_string" +require_relative "./multiline_to_string" + +module RailInspector + module Visitor + class FrameworkDefault + TargetVersionCaseFinder = + SyntaxTree::Search.new( + ->(node) do + node in SyntaxTree::Case[ + value: SyntaxTree::CallNode[ + receiver: SyntaxTree::VarRef[ + value: SyntaxTree::Ident[value: "target_version"] + ] + ] + ] + end + ) + + attr_reader :config_map + + def initialize + @config_map = {} + end + + def visit(node) + case_node, *others = TargetVersionCaseFinder.scan(node).to_a + raise "#{others.length} other cases?" unless others.empty? + + visit_when(case_node.consequent) + end + + private + def visit_when(node) + version = node.arguments.parts[0].parts[0].value + + config_map[version] = VersionedConfig.new.config_for(node.statements) + + visit_when(node.consequent) if node.consequent.is_a? SyntaxTree::When + end + + class VersionedConfig < SyntaxTree::Visitor + attr_reader :configs + + def initialize + @configs = {} + @framework_stack = [] + end + + def config_for(node) + visit(node) + @configs + end + + visit_methods do + def visit_if(node) + unless new_framework = respond_to_framework?(node.predicate) + return super + end + + if ENV["STRICT"] && current_framework + raise "Potentially nested framework? Current: '#{current_framework}', found: '#{new_framework}'" + end + + @framework_stack << new_framework + super + @framework_stack.pop + end + + def visit_assign(node) + assert_framework(node) + + target = SyntaxTree::Formatter.format(nil, node.target) + value = + case node.value + when SyntaxTree::HashLiteral + HashToString.new.tap { |v| v.visit(node.value) }.to_s + when SyntaxTree::StringConcat + MultilineToString.new.tap { |v| v.visit(node.value) }.to_s + else + SyntaxTree::Formatter.format(nil, node.value) + end + @configs[target] = value + end + end + + private + def assert_framework(node) + framework = + case node.target.parent + in { value: SyntaxTree::Const } | + { value: SyntaxTree::Kw[value: "self"] } + nil + in receiver: { value: { value: framework } } + framework + in value: { value: framework } + framework + end + + return if current_framework == framework + + raise "Expected #{current_framework} to match #{framework}" + end + + def current_framework + @framework_stack.last + end + + def respond_to_framework?(node) + if node in SyntaxTree::CallNode[ + message: SyntaxTree::Ident[value: "respond_to?"], + arguments: SyntaxTree::ArgParen[ + arguments: SyntaxTree::Args[ + parts: [ + SyntaxTree::SymbolLiteral[ + value: SyntaxTree::Ident[value: new_framework] + ] + ] + ] + ] + ] + new_framework + end + end + end + end + end +end diff --git a/tools/rail_inspector/lib/rail_inspector/visitor/hash_to_string.rb b/tools/rail_inspector/lib/rail_inspector/visitor/hash_to_string.rb new file mode 100644 index 00000000000..0b88bf875d0 --- /dev/null +++ b/tools/rail_inspector/lib/rail_inspector/visitor/hash_to_string.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require "syntax_tree" + +module RailInspector + module Visitor + class HashToString < SyntaxTree::Visitor + attr_reader :to_s + + def initialize + @to_s = +"" + end + + visit_methods do + def visit_assoc(node) + @to_s << " " + visit(node.key) + + case node.key + when SyntaxTree::StringLiteral + @to_s << " => " + end + + visit(node.value) + end + + def visit_hash(node) + @to_s << "{" + + if node.assocs.length > 0 + visit(node.assocs[0]) + + if node.assocs.length > 1 + node.assocs[1..-1].each do |a| + @to_s << "," + visit(a) + end + end + @to_s << " " + end + + @to_s << "}" + end + + def visit_int(node) + @to_s << node.value + end + + def visit_kw(node) + @to_s << node.value + end + + def visit_label(node) + @to_s << node.value + @to_s << " " + end + + def visit_tstring_content(node) + @to_s << '"' + @to_s << node.value + @to_s << '"' + end + end + end + end +end diff --git a/tools/rail_inspector/lib/rail_inspector/visitor/multiline_to_string.rb b/tools/rail_inspector/lib/rail_inspector/visitor/multiline_to_string.rb new file mode 100644 index 00000000000..64aa0d60e97 --- /dev/null +++ b/tools/rail_inspector/lib/rail_inspector/visitor/multiline_to_string.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require "syntax_tree" + +module RailInspector + module Visitor + class MultilineToString < SyntaxTree::Visitor + attr_reader :to_s + + def initialize + @to_s = +"" + end + + visit_methods do + def visit_string_concat(node) + @to_s << '"' + super(node) + @to_s << '"' + end + + def visit_tstring_content(node) + @to_s << node.value + end + end + end + end +end diff --git a/tools/rail_inspector/rail_inspector.gemspec b/tools/rail_inspector/rail_inspector.gemspec new file mode 100644 index 00000000000..1655e34810d --- /dev/null +++ b/tools/rail_inspector/rail_inspector.gemspec @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require_relative "lib/rail_inspector/version" + +Gem::Specification.new do |spec| + spec.name = "rail_inspector" + spec.version = RailInspector::VERSION + spec.authors = ["Hartley McGuire"] + spec.email = ["skipkayhil@gmail.com"] + + spec.summary = "A collection of linters for rails/rails" + spec.homepage = "https://github.com/skipkayhil/rail_inspector" + spec.license = "MIT" + spec.required_ruby_version = ">= 2.7.0" + + spec.metadata["homepage_uri"] = spec.homepage + spec.metadata["source_code_uri"] = spec.homepage + # spec.metadata["changelog_uri"] = "TODO: Put your gem's CHANGELOG.md URL here." + + # Specify which files should be added to the gem when it is released. + # The `git ls-files -z` loads the files in the RubyGem that have been added into git. + spec.files = Dir.chdir(__dir__) do + `git ls-files -z`.split("\x0").reject do |f| + (File.expand_path(f) == __FILE__) || f.start_with?(*%w[bin/ test/ spec/ features/ .git .circleci appveyor]) + end + end + spec.bindir = "exe" + spec.executables = spec.files.grep(%r{\Aexe/}) { |f| File.basename(f) } + spec.require_paths = ["lib"] + + # Uncomment to register a new dependency of your gem + spec.add_dependency "syntax_tree", "6.1.1" + spec.add_dependency "thor", "~> 1.0" +end diff --git a/tools/rail_inspector/test/fixtures/action_mailbox_83d85b2.md b/tools/rail_inspector/test/fixtures/action_mailbox_83d85b2.md new file mode 100644 index 00000000000..0bb4a4e8e32 --- /dev/null +++ b/tools/rail_inspector/test/fixtures/action_mailbox_83d85b2.md @@ -0,0 +1,3 @@ + + +Please check [7-0-stable](https://github.com/rails/rails/blob/7-0-stable/actionmailbox/CHANGELOG.md) for previous changes. diff --git a/tools/rail_inspector/test/fixtures/action_pack_69d504.md b/tools/rail_inspector/test/fixtures/action_pack_69d504.md new file mode 100644 index 00000000000..d72abba38d2 --- /dev/null +++ b/tools/rail_inspector/test/fixtures/action_pack_69d504.md @@ -0,0 +1,10 @@ +* This entry is not real but triggers the error because the next header is + incorrectly ambiguous when it isn't the top entry + + *Hartley McGuire* + +* Expand search field on `rails/info/routes` to also search **route name**, **http verb** and **controller#action** + + *Jason Kotchoff* + +Please check [7-0-stable](https://github.com/rails/rails/blob/7-0-stable/actionpack/CHANGELOG.md) for previous changes. diff --git a/tools/rail_inspector/test/fixtures/active_record_238432d.md b/tools/rail_inspector/test/fixtures/active_record_238432d.md new file mode 100644 index 00000000000..3dc16286673 --- /dev/null +++ b/tools/rail_inspector/test/fixtures/active_record_238432d.md @@ -0,0 +1,300 @@ +* Add ability to ignore tables by regexp for SQL schema dumps. + + ```ruby + ActiveRecord::SchemaDumper.ignore_tables = [/^_/] + ``` + + *fatkodima* + +* Avoid queries when performing calculations on contradictory relations. + + Previously calculations would make a query even when passed a + contradiction, such as `User.where(id: []).count`. We no longer perform a + query in that scenario. + + This applies to the following calculations: `count`, `sum`, `average`, + `minimum` and `maximum` + + *Luan Vieira, John Hawthorn and Daniel Colson* + +* Allow using aliased attributes with `insert_all`/`upsert_all`. + + ```ruby + class Book < ApplicationRecord + alias_attribute :title, :name + end + + Book.insert_all [{ title: "Remote", author_id: 1 }], returning: :title + ``` + + *fatkodima* + +* Support encrypted attributes on columns with default db values. + +This adds support for encrypted attributes defined on columns with default values. +It will encrypt those values at creation time. Before, it would raise an +error unless `config.active_record.encryption.support_unencrypted_data` was true. + +*Jorge Manrubia* and *Dima Fatko* + +* Allow overriding `reading_request?` in `DatabaseSelector::Resolver` + + The default implementation checks if a request is a `get?` or `head?`, + but you can now change it to anything you like. If the method returns true, + `Resolver#read` gets called meaning the request could be served by the + replica database. + + *Alex Ghiculescu* + +* Remove `ActiveRecord.legacy_connection_handling`. + + *Eileen M. Uchitelle* + +* `rails db:schema:{dump,load}` now checks `ENV["SCHEMA_FORMAT"]` before config + + Since `rails db:structure:{dump,load}` was deprecated there wasn't a simple + way to dump a schema to both SQL and Ruby formats. You can now do this with + an environment variable. For example: + + ``` + SCHEMA_FORMAT=sql rake db:schema:dump + ``` + + *Alex Ghiculescu* + +* Fixed MariaDB default function support. + + Defaults would be written wrong in "db/schema.rb" and not work correctly + if using `db:schema:load`. Further more the function name would be + added as string content when saving new records. + + *kaspernj* + +* Add `active_record.destroy_association_async_batch_size` configuration + + This allows applications to specify the maximum number of records that will + be destroyed in a single background job by the `dependent: :destroy_async` + association option. By default, the current behavior will remain the same: + when a parent record is destroyed, all dependent records will be destroyed + in a single background job. If the number of dependent records is greater + than this configuration, the records will be destroyed in multiple + background jobs. + + *Nick Holden* + +* Fix `remove_foreign_key` with `:if_exists` option when foreign key actually exists. + + *fatkodima* + +* Remove `--no-comments` flag in structure dumps for PostgreSQL + + This broke some apps that used custom schema comments. If you don't want + comments in your structure dump, you can use: + + ```ruby + ActiveRecord::Tasks::DatabaseTasks.structure_dump_flags = ['--no-comments'] + ``` + + *Alex Ghiculescu* + +* Reduce the memory footprint of fixtures accessors. + + Until now fixtures accessors were eagerly defined using `define_method`. + So the memory usage was directly dependent of the number of fixtures and + test suites. + + Instead fixtures accessors are now implemented with `method_missing`, + so they incur much less memory and CPU overhead. + + *Jean Boussier* + +* Fix `config.active_record.destroy_association_async_job` configuration + + `config.active_record.destroy_association_async_job` should allow + applications to specify the job that will be used to destroy associated + records in the background for `has_many` associations with the + `dependent: :destroy_async` option. Previously, that was ignored, which + meant the default `ActiveRecord::DestroyAssociationAsyncJob` always + destroyed records in the background. + + *Nick Holden* + +* Fix `change_column_comment` to preserve column's AUTO_INCREMENT in the MySQL adapter + + *fatkodima* + +* Fix quoting of `ActiveSupport::Duration` and `Rational` numbers in the MySQL adapter. + + *Kevin McPhillips* + +* Allow column name with COLLATE (e.g., title COLLATE "C") as safe SQL string + + *Shugo Maeda* + +* Permit underscores in the VERSION argument to database rake tasks. + + *Eddie Lebow* + +* Reversed the order of `INSERT` statements in `structure.sql` dumps + + This should decrease the likelihood of merge conflicts. New migrations + will now be added at the top of the list. + + For existing apps, there will be a large diff the next time `structure.sql` + is generated. + + *Alex Ghiculescu*, *Matt Larraz* + +* Fix PG.connect keyword arguments deprecation warning on ruby 2.7 + + Fixes #44307. + + *Nikita Vasilevsky* + +* Fix dropping DB connections after serialization failures and deadlocks. + + Prior to 6.1.4, serialization failures and deadlocks caused rollbacks to be + issued for both real transactions and savepoints. This breaks MySQL which + disallows rollbacks of savepoints following a deadlock. + + 6.1.4 removed these rollbacks, for both transactions and savepoints, causing + the DB connection to be left in an unknown state and thus discarded. + + These rollbacks are now restored, except for savepoints on MySQL. + + *Thomas Morgan* + +* Make `ActiveRecord::ConnectionPool` Fiber-safe + + When `ActiveSupport::IsolatedExecutionState.isolation_level` is set to `:fiber`, + the connection pool now supports multiple Fibers from the same Thread checking + out connections from the pool. + + *Alex Matchneer* + +* Add `update_attribute!` to `ActiveRecord::Persistence` + + Similar to `update_attribute`, but raises `ActiveRecord::RecordNotSaved` when a `before_*` callback throws `:abort`. + + ```ruby + class Topic < ActiveRecord::Base + before_save :check_title + + def check_title + throw(:abort) if title == "abort" + end + end + + topic = Topic.create(title: "Test Title") + # #=> # + topic.update_attribute!(:title, "Another Title") + # #=> # + topic.update_attribute!(:title, "abort") + # raises ActiveRecord::RecordNotSaved + ``` + + *Drew Tempelmeyer* + +* Avoid loading every record in `ActiveRecord::Relation#pretty_print` + + ```ruby + # Before + pp Foo.all # Loads the whole table. + + # After + pp Foo.all # Shows 10 items and an ellipsis. + ``` + + *Ulysse Buonomo* + +* Change `QueryMethods#in_order_of` to drop records not listed in values. + + `in_order_of` now filters down to the values provided, to match the behavior of the `Enumerable` version. + + *Kevin Newton* + +* Allow named expression indexes to be revertible. + + Previously, the following code would raise an error in a reversible migration executed while rolling back, due to the index name not being used in the index removal. + + ```ruby + add_index(:settings, "(data->'property')", using: :gin, name: :index_settings_data_property) + ``` + + Fixes #43331. + + *Oliver Günther* + +* Fix incorrect argument in PostgreSQL structure dump tasks. + + Updating the `--no-comment` argument added in Rails 7 to the correct `--no-comments` argument. + + *Alex Dent* + +* Fix migration compatibility to create SQLite references/belongs_to column as integer when migration version is 6.0. + + Reference/belongs_to in migrations with version 6.0 were creating columns as + bigint instead of integer for the SQLite Adapter. + + *Marcelo Lauxen* + +* Add a deprecation warning when `prepared_statements` configuration is not + set for the mysql2 adapter. + + *Thiago Araujo and Stefanni Brasil* + +* Fix `QueryMethods#in_order_of` to handle empty order list. + + ```ruby + Post.in_order_of(:id, []).to_a + ``` + + Also more explicitly set the column as secondary order, so that any other + value is still ordered. + + *Jean Boussier* + +* Fix quoting of column aliases generated by calculation methods. + + Since the alias is derived from the table name, we can't assume the result + is a valid identifier. + + ```ruby + class Test < ActiveRecord::Base + self.table_name = '1abc' + end + Test.group(:id).count + # syntax error at or near "1" (ActiveRecord::StatementInvalid) + # LINE 1: SELECT COUNT(*) AS count_all, "1abc"."id" AS 1abc_id FROM "1... + ``` + + *Jean Boussier* + +* Add `authenticate_by` when using `has_secure_password`. + + `authenticate_by` is intended to replace code like the following, which + returns early when a user with a matching email is not found: + + ```ruby + User.find_by(email: "...")&.authenticate("...") + ``` + + Such code is vulnerable to timing-based enumeration attacks, wherein an + attacker can determine if a user account with a given email exists. After + confirming that an account exists, the attacker can try passwords associated + with that email address from other leaked databases, in case the user + re-used a password across multiple sites (a common practice). Additionally, + knowing an account email address allows the attacker to attempt a targeted + phishing ("spear phishing") attack. + + `authenticate_by` addresses the vulnerability by taking the same amount of + time regardless of whether a user with a matching email is found: + + ```ruby + User.authenticate_by(email: "...", password: "...") + ``` + + *Jonathan Hefner* + + +Please check [7-0-stable](https://github.com/rails/rails/blob/7-0-stable/activerecord/CHANGELOG.md) for previous changes. diff --git a/tools/rail_inspector/test/fixtures/active_record_51852d2.md b/tools/rail_inspector/test/fixtures/active_record_51852d2.md new file mode 100644 index 00000000000..e0572ba7490 --- /dev/null +++ b/tools/rail_inspector/test/fixtures/active_record_51852d2.md @@ -0,0 +1,190 @@ +* Fix `config.active_record.destroy_association_async_job` configuration + + `config.active_record.destroy_association_async_job` should allow + applications to specify the job that will be used to destroy associated + records in the background for `has_many` associations with the + `dependent: :destroy_async` option. Previously, that was ignored, which + meant the default `ActiveRecord::DestroyAssociationAsyncJob` always + destroyed records in the background. + + *Nick Holden* + +* Fix `change_column_comment` to preserve column's AUTO_INCREMENT in the MySQL adapter + + *fatkodima* + +* Fix quoting of `ActiveSupport::Duration` and `Rational` numbers in the MySQL adapter. + + *Kevin McPhillips* + +* Allow column name with COLLATE (e.g., title COLLATE "C") as safe SQL string + + *Shugo Maeda* + +* Permit underscores in the VERSION argument to database rake tasks. + + *Eddie Lebow* + +* Reversed the order of `INSERT` statements in `structure.sql` dumps + + This should decrease the likelihood of merge conflicts. New migrations + will now be added at the top of the list. + + For existing apps, there will be a large diff the next time `structure.sql` + is generated. + + *Alex Ghiculescu*, *Matt Larraz* + +* Fix PG.connect keyword arguments deprecation warning on ruby 2.7 + + Fixes #44307. + + *Nikita Vasilevsky* + +* Fix dropping DB connections after serialization failures and deadlocks. + + Prior to 6.1.4, serialization failures and deadlocks caused rollbacks to be + issued for both real transactions and savepoints. This breaks MySQL which + disallows rollbacks of savepoints following a deadlock. + + 6.1.4 removed these rollbacks, for both transactions and savepoints, causing + the DB connection to be left in an unknown state and thus discarded. + + These rollbacks are now restored, except for savepoints on MySQL. + + *Thomas Morgan* + +* Make `ActiveRecord::ConnectionPool` Fiber-safe + + When `ActiveSupport::IsolatedExecutionState.isolation_level` is set to `:fiber`, + the connection pool now supports multiple Fibers from the same Thread checking + out connections from the pool. + + *Alex Matchneer* + +* Add `update_attribute!` to `ActiveRecord::Persistence` + + Similar to `update_attribute`, but raises `ActiveRecord::RecordNotSaved` when a `before_*` callback throws `:abort`. + + ```ruby + class Topic < ActiveRecord::Base + before_save :check_title + + def check_title + throw(:abort) if title == "abort" + end + end + + topic = Topic.create(title: "Test Title") + # #=> # + topic.update_attribute!(:title, "Another Title") + # #=> # + topic.update_attribute!(:title, "abort") + # raises ActiveRecord::RecordNotSaved + ``` + + *Drew Tempelmeyer* + +* Avoid loading every record in `ActiveRecord::Relation#pretty_print` + + ```ruby + # Before + pp Foo.all # Loads the whole table. + + # After + pp Foo.all # Shows 10 items and an ellipsis. + ``` + + *Ulysse Buonomo* + +* Change `QueryMethods#in_order_of` to drop records not listed in values. + + `in_order_of` now filters down to the values provided, to match the behavior of the `Enumerable` version. + + *Kevin Newton* + +* Allow named expression indexes to be revertible. + + Previously, the following code would raise an error in a reversible migration executed while rolling back, due to the index name not being used in the index removal. + + ```ruby + add_index(:settings, "(data->'property')", using: :gin, name: :index_settings_data_property) + ``` + + Fixes #43331. + + *Oliver Günther* + +* Fix incorrect argument in PostgreSQL structure dump tasks. + + Updating the `--no-comment` argument added in Rails 7 to the correct `--no-comments` argument. + + *Alex Dent* + +* Fix migration compatibility to create SQLite references/belongs_to column as integer when migration version is 6.0. + + Reference/belongs_to in migrations with version 6.0 were creating columns as + bigint instead of integer for the SQLite Adapter. + + *Marcelo Lauxen* + +* Add a deprecation warning when `prepared_statements` configuration is not + set for the mysql2 adapter. + + *Thiago Araujo and Stefanni Brasil* + +* Fix `QueryMethods#in_order_of` to handle empty order list. + + ```ruby + Post.in_order_of(:id, []).to_a + ``` + + Also more explicitly set the column as secondary order, so that any other + value is still ordered. + + *Jean Boussier* + +* Fix quoting of column aliases generated by calculation methods. + + Since the alias is derived from the table name, we can't assume the result + is a valid identifier. + + ```ruby + class Test < ActiveRecord::Base + self.table_name = '1abc' + end + Test.group(:id).count + # syntax error at or near "1" (ActiveRecord::StatementInvalid) + # LINE 1: SELECT COUNT(*) AS count_all, "1abc"."id" AS 1abc_id FROM "1... + ``` + + *Jean Boussier* + +* Add `authenticate_by` when using `has_secure_password`. + + `authenticate_by` is intended to replace code like the following, which + returns early when a user with a matching email is not found: + + ```ruby + User.find_by(email: "...")&.authenticate("...") + ``` + + Such code is vulnerable to timing-based enumeration attacks, wherein an + attacker can determine if a user account with a given email exists. After + confirming that an account exists, the attacker can try passwords associated + with that email address from other leaked databases, in case the user + re-used a password across multiple sites (a common practice). Additionally, + knowing an account email address allows the attacker to attempt a targeted + phishing ("spear phishing") attack. + + `authenticate_by` addresses the vulnerability by taking the same amount of + time regardless of whether a user with a matching email is found: + + ```ruby + User.authenticate_by(email: "...", password: "...") + ``` + + *Jonathan Hefner* + + +Please check [7-0-stable](https://github.com/rails/rails/blob/7-0-stable/activerecord/CHANGELOG.md) for previous changes. diff --git a/tools/rail_inspector/test/fixtures/active_record_936a862.md b/tools/rail_inspector/test/fixtures/active_record_936a862.md new file mode 100644 index 00000000000..925a1a37428 --- /dev/null +++ b/tools/rail_inspector/test/fixtures/active_record_936a862.md @@ -0,0 +1,476 @@ +* Introduce strategy pattern for executing migrations. + + By default, migrations will use a strategy object that delegates the method + to the connection adapter. Consumers can implement custom strategy objects + to change how their migrations run. + + *Adrianna Chang* + +* Add adapter option disallowing foreign keys + + This adds a new option to be added to `database.yml` which enables skipping + foreign key constraints usage even if the underlying database supports them. + + Usage: + ```yaml + development: + <<: *default + database: db/development.sqlite3 + foreign_keys: false + ``` + + *Paulo Barros* + +* Add configurable deprecation warning for singular associations + + This adds a deprecation warning when using the plural name of a singular associations in `where`. + It is possible to opt into the new more performant behavior with `config.active_record.allow_deprecated_singular_associations_name = false` + + *Adam Hess* + +* Run transactional callbacks on the freshest instance to save a given + record within a transaction. + + When multiple Active Record instances change the same record within a + transaction, Rails runs `after_commit` or `after_rollback` callbacks for + only one of them. `config.active_record.run_commit_callbacks_on_first_saved_instances_in_transaction` + was added to specify how Rails chooses which instance receives the + callbacks. The framework defaults were changed to use the new logic. + + When `config.active_record.run_commit_callbacks_on_first_saved_instances_in_transaction` + is `true`, transactional callbacks are run on the first instance to save, + even though its instance state may be stale. + + When it is `false`, which is the new framework default starting with version + 7.1, transactional callbacks are run on the instances with the freshest + instance state. Those instances are chosen as follows: + + - In general, run transactional callbacks on the last instance to save a + given record within the transaction. + - There are two exceptions: + - If the record is created within the transaction, then updated by + another instance, `after_create_commit` callbacks will be run on the + second instance. This is instead of the `after_update_commit` + callbacks that would naively be run based on that instance’s state. + - If the record is destroyed within the transaction, then + `after_destroy_commit` callbacks will be fired on the last destroyed + instance, even if a stale instance subsequently performed an update + (which will have affected 0 rows). + + *Cameron Bothner and Mitch Vollebregt* + +* Enable strict strings mode for `SQLite3Adapter`. + + Configures SQLite with a strict strings mode, which disables double-quoted string literals. + + SQLite has some quirks around double-quoted string literals. + It first tries to consider double-quoted strings as identifier names, but if they don't exist + it then considers them as string literals. Because of this, typos can silently go unnoticed. + For example, it is possible to create an index for a non existing column. + See [SQLite documentation](https://www.sqlite.org/quirks.html#double_quoted_string_literals_are_accepted) for more details. + + If you don't want this behavior, you can disable it via: + + ```ruby + # config/application.rb + config.active_record.sqlite3_adapter_strict_strings_by_default = false + ``` + + Fixes #27782. + + *fatkodima*, *Jean Boussier* + +* Resolve issue where a relation cache_version could be left stale. + + Previously, when `reset` was called on a relation object it did not reset the cache_versions + ivar. This led to a confusing situation where despite having the correct data the relation + still reported a stale cache_version. + + Usage: + + ```ruby + developers = Developer.all + developers.cache_version + + Developer.update_all(updated_at: Time.now.utc + 1.second) + + developers.cache_version # Stale cache_version + developers.reset + developers.cache_version # Returns the current correct cache_version + ``` + + Fixes #45341. + + *Austen Madden* + +* Add support for exclusion constraints (PostgreSQL-only). + + ```ruby + add_exclusion_constraint :invoices, "daterange(start_date, end_date) WITH &&", using: :gist, name: "invoices_date_overlap" + remove_exclusion_constraint :invoices, name: "invoices_date_overlap" + ``` + + See PostgreSQL's [`CREATE TABLE ... EXCLUDE ...`](https://www.postgresql.org/docs/12/sql-createtable.html#SQL-CREATETABLE-EXCLUDE) documentation for more on exclusion constraints. + + *Alex Robbin* + +* `change_column_null` raises if a non-boolean argument is provided + + Previously if you provided a non-boolean argument, `change_column_null` would + treat it as truthy and make your column nullable. This could be surprising, so now + the input must be either `true` or `false`. + + ```ruby + change_column_null :table, :column, true # good + change_column_null :table, :column, false # good + change_column_null :table, :column, from: true, to: false # raises (previously this made the column nullable) + ``` + + *Alex Ghiculescu* + +* Enforce limit on table names length. + + Fixes #45130. + + *fatkodima* + +* Adjust the minimum MariaDB version for check constraints support. + + *Eddie Lebow* + +* Fix Hstore deserialize regression. + + *edsharp* + +* Add validity for PostgreSQL indexes. + + ```ruby + connection.index_exists?(:users, :email, valid: true) + connection.indexes(:users).select(&:valid?) + ``` + + *fatkodima* + +* Fix eager loading for models without primary keys. + + *Anmol Chopra*, *Matt Lawrence*, and *Jonathan Hefner* + +* Avoid validating a unique field if it has not changed and is backed by a unique index. + + Previously, when saving a record, ActiveRecord will perform an extra query to check for the uniqueness of + each attribute having a `uniqueness` validation, even if that attribute hasn't changed. + If the database has the corresponding unique index, then this validation can never fail for persisted records, + and we could safely skip it. + + *fatkodima* + +* Stop setting `sql_auto_is_null` + + Since version 5.5 the default has been off, we no longer have to manually turn it off. + + *Adam Hess* + +* Fix `touch` to raise an error for readonly columns. + + *fatkodima* + +* Add ability to ignore tables by regexp for SQL schema dumps. + + ```ruby + ActiveRecord::SchemaDumper.ignore_tables = [/^_/] + ``` + + *fatkodima* + +* Avoid queries when performing calculations on contradictory relations. + + Previously calculations would make a query even when passed a + contradiction, such as `User.where(id: []).count`. We no longer perform a + query in that scenario. + + This applies to the following calculations: `count`, `sum`, `average`, + `minimum` and `maximum` + + *Luan Vieira, John Hawthorn and Daniel Colson* + +* Allow using aliased attributes with `insert_all`/`upsert_all`. + + ```ruby + class Book < ApplicationRecord + alias_attribute :title, :name + end + + Book.insert_all [{ title: "Remote", author_id: 1 }], returning: :title + ``` + + *fatkodima* + +* Support encrypted attributes on columns with default db values. + + This adds support for encrypted attributes defined on columns with default values. + It will encrypt those values at creation time. Before, it would raise an + error unless `config.active_record.encryption.support_unencrypted_data` was true. + + *Jorge Manrubia* and *Dima Fatko* + +* Allow overriding `reading_request?` in `DatabaseSelector::Resolver` + + The default implementation checks if a request is a `get?` or `head?`, + but you can now change it to anything you like. If the method returns true, + `Resolver#read` gets called meaning the request could be served by the + replica database. + + *Alex Ghiculescu* + +* Remove `ActiveRecord.legacy_connection_handling`. + + *Eileen M. Uchitelle* + +* `rails db:schema:{dump,load}` now checks `ENV["SCHEMA_FORMAT"]` before config + + Since `rails db:structure:{dump,load}` was deprecated there wasn't a simple + way to dump a schema to both SQL and Ruby formats. You can now do this with + an environment variable. For example: + + ``` + SCHEMA_FORMAT=sql rake db:schema:dump + ``` + + *Alex Ghiculescu* + +* Fixed MariaDB default function support. + + Defaults would be written wrong in "db/schema.rb" and not work correctly + if using `db:schema:load`. Further more the function name would be + added as string content when saving new records. + + *kaspernj* + +* Add `active_record.destroy_association_async_batch_size` configuration + + This allows applications to specify the maximum number of records that will + be destroyed in a single background job by the `dependent: :destroy_async` + association option. By default, the current behavior will remain the same: + when a parent record is destroyed, all dependent records will be destroyed + in a single background job. If the number of dependent records is greater + than this configuration, the records will be destroyed in multiple + background jobs. + + *Nick Holden* + +* Fix `remove_foreign_key` with `:if_exists` option when foreign key actually exists. + + *fatkodima* + +* Remove `--no-comments` flag in structure dumps for PostgreSQL + + This broke some apps that used custom schema comments. If you don't want + comments in your structure dump, you can use: + + ```ruby + ActiveRecord::Tasks::DatabaseTasks.structure_dump_flags = ['--no-comments'] + ``` + + *Alex Ghiculescu* + +* Reduce the memory footprint of fixtures accessors. + + Until now fixtures accessors were eagerly defined using `define_method`. + So the memory usage was directly dependent of the number of fixtures and + test suites. + + Instead fixtures accessors are now implemented with `method_missing`, + so they incur much less memory and CPU overhead. + + *Jean Boussier* + +* Fix `config.active_record.destroy_association_async_job` configuration + + `config.active_record.destroy_association_async_job` should allow + applications to specify the job that will be used to destroy associated + records in the background for `has_many` associations with the + `dependent: :destroy_async` option. Previously, that was ignored, which + meant the default `ActiveRecord::DestroyAssociationAsyncJob` always + destroyed records in the background. + + *Nick Holden* + +* Fix `change_column_comment` to preserve column's AUTO_INCREMENT in the MySQL adapter + + *fatkodima* + +* Fix quoting of `ActiveSupport::Duration` and `Rational` numbers in the MySQL adapter. + + *Kevin McPhillips* + +* Allow column name with COLLATE (e.g., title COLLATE "C") as safe SQL string + + *Shugo Maeda* + +* Permit underscores in the VERSION argument to database rake tasks. + + *Eddie Lebow* + +* Reversed the order of `INSERT` statements in `structure.sql` dumps + + This should decrease the likelihood of merge conflicts. New migrations + will now be added at the top of the list. + + For existing apps, there will be a large diff the next time `structure.sql` + is generated. + + *Alex Ghiculescu*, *Matt Larraz* + +* Fix PG.connect keyword arguments deprecation warning on ruby 2.7 + + Fixes #44307. + + *Nikita Vasilevsky* + +* Fix dropping DB connections after serialization failures and deadlocks. + + Prior to 6.1.4, serialization failures and deadlocks caused rollbacks to be + issued for both real transactions and savepoints. This breaks MySQL which + disallows rollbacks of savepoints following a deadlock. + + 6.1.4 removed these rollbacks, for both transactions and savepoints, causing + the DB connection to be left in an unknown state and thus discarded. + + These rollbacks are now restored, except for savepoints on MySQL. + + *Thomas Morgan* + +* Make `ActiveRecord::ConnectionPool` Fiber-safe + + When `ActiveSupport::IsolatedExecutionState.isolation_level` is set to `:fiber`, + the connection pool now supports multiple Fibers from the same Thread checking + out connections from the pool. + + *Alex Matchneer* + +* Add `update_attribute!` to `ActiveRecord::Persistence` + + Similar to `update_attribute`, but raises `ActiveRecord::RecordNotSaved` when a `before_*` callback throws `:abort`. + + ```ruby + class Topic < ActiveRecord::Base + before_save :check_title + + def check_title + throw(:abort) if title == "abort" + end + end + + topic = Topic.create(title: "Test Title") + # #=> # + topic.update_attribute!(:title, "Another Title") + # #=> # + topic.update_attribute!(:title, "abort") + # raises ActiveRecord::RecordNotSaved + ``` + + *Drew Tempelmeyer* + +* Avoid loading every record in `ActiveRecord::Relation#pretty_print` + + ```ruby + # Before + pp Foo.all # Loads the whole table. + + # After + pp Foo.all # Shows 10 items and an ellipsis. + ``` + + *Ulysse Buonomo* + +* Change `QueryMethods#in_order_of` to drop records not listed in values. + + `in_order_of` now filters down to the values provided, to match the behavior of the `Enumerable` version. + + *Kevin Newton* + +* Allow named expression indexes to be revertible. + + Previously, the following code would raise an error in a reversible migration executed while rolling back, due to the index name not being used in the index removal. + + ```ruby + add_index(:settings, "(data->'property')", using: :gin, name: :index_settings_data_property) + ``` + + Fixes #43331. + + *Oliver Günther* + +* Fix incorrect argument in PostgreSQL structure dump tasks. + + Updating the `--no-comment` argument added in Rails 7 to the correct `--no-comments` argument. + + *Alex Dent* + +* Fix migration compatibility to create SQLite references/belongs_to column as integer when migration version is 6.0. + + Reference/belongs_to in migrations with version 6.0 were creating columns as + bigint instead of integer for the SQLite Adapter. + + *Marcelo Lauxen* + +* Add a deprecation warning when `prepared_statements` configuration is not + set for the mysql2 adapter. + + *Thiago Araujo and Stefanni Brasil* + +* Fix `QueryMethods#in_order_of` to handle empty order list. + + ```ruby + Post.in_order_of(:id, []).to_a + ``` + + Also more explicitly set the column as secondary order, so that any other + value is still ordered. + + *Jean Boussier* + +* Fix quoting of column aliases generated by calculation methods. + + Since the alias is derived from the table name, we can't assume the result + is a valid identifier. + + ```ruby + class Test < ActiveRecord::Base + self.table_name = '1abc' + end + Test.group(:id).count + # syntax error at or near "1" (ActiveRecord::StatementInvalid) + # LINE 1: SELECT COUNT(*) AS count_all, "1abc"."id" AS 1abc_id FROM "1... + ``` + + *Jean Boussier* + +* Add `authenticate_by` when using `has_secure_password`. + + `authenticate_by` is intended to replace code like the following, which + returns early when a user with a matching email is not found: + + ```ruby + User.find_by(email: "...")&.authenticate("...") + ``` + + Such code is vulnerable to timing-based enumeration attacks, wherein an + attacker can determine if a user account with a given email exists. After + confirming that an account exists, the attacker can try passwords associated + with that email address from other leaked databases, in case the user + re-used a password across multiple sites (a common practice). Additionally, + knowing an account email address allows the attacker to attempt a targeted + phishing ("spear phishing") attack. + + `authenticate_by` addresses the vulnerability by taking the same amount of + time regardless of whether a user with a matching email is found: + + ```ruby + User.authenticate_by(email: "...", password: "...") + ``` + + *Jonathan Hefner* + + +Please check [7-0-stable](https://github.com/rails/rails/blob/7-0-stable/activerecord/CHANGELOG.md) for previous changes. diff --git a/tools/rail_inspector/test/fixtures/active_support_9f0b8eb.md b/tools/rail_inspector/test/fixtures/active_support_9f0b8eb.md new file mode 100644 index 00000000000..b3aa9dca3d0 --- /dev/null +++ b/tools/rail_inspector/test/fixtures/active_support_9f0b8eb.md @@ -0,0 +1,152 @@ +* Fix `NoMethodError` on custom `ActiveSupport::Deprecation` behavior. + + `ActiveSupport::Deprecation.behavior=` was supposed to accept any object + that responds to `call`, but in fact its internal implementation assumed that + this object could respond to `arity`, so it was restricted to only `Proc` objects. + + This change removes this `arity` restriction of custom behaviors. + + *Ryo Nakamura* + +* Support `:url_safe` option for `MessageEncryptor`. + + The `MessageEncryptor` constructor now accepts a `:url_safe` option, similar + to the `MessageVerifier` constructor. When enabled, this option ensures + that messages use a URL-safe encoding. + + *Jonathan Hefner* + +* Add `url_safe` option to `ActiveSupport::MessageVerifier` initializer + + `ActiveSupport::MessageVerifier.new` now takes optional `url_safe` argument. + It can generate URL-safe strings by passing `url_safe: true`. + + ```ruby + verifier = ActiveSupport::MessageVerifier.new(url_safe: true) + message = verifier.generate(data) # => URL-safe string + ``` + + This option is `false` by default to be backwards compatible. + + *Shouichi Kamiya* + +* Enable connection pooling by default for `MemCacheStore` and `RedisCacheStore`. + + If you want to disable connection pooling, set `:pool` option to `false` when configuring the cache store: + + ```ruby + config.cache_store = :mem_cache_store, "cache.example.com", pool: false + ``` + + *fatkodima* + +* Add `force:` support to `ActiveSupport::Cache::Store#fetch_multi`. + + *fatkodima* + +* Deprecated `:pool_size` and `:pool_timeout` options for configuring connection pooling in cache stores. + + Use `pool: true` to enable pooling with default settings: + + ```ruby + config.cache_store = :redis_cache_store, pool: true + ``` + + Or pass individual options via `:pool` option: + + ```ruby + config.cache_store = :redis_cache_store, pool: { size: 10, timeout: 2 } + ``` + + *fatkodima* + +* Allow #increment and #decrement methods of `ActiveSupport::Cache::Store` + subclasses to set new values. + + Previously incrementing or decrementing an unset key would fail and return + nil. A default will now be assumed and the key will be created. + + *Andrej Blagojević*, *Eugene Kenny* + +* Add `skip_nil:` support to `RedisCacheStore` + + *Joey Paris* + +* `ActiveSupport::Cache::MemoryStore#write(name, val, unless_exist:true)` now + correctly writes expired keys. + + *Alan Savage* + +* `ActiveSupport::ErrorReporter` now accepts and forward a `source:` parameter. + + This allow libraries to signal the origin of the errors, and reporters + to easily ignore some sources. + + *Jean Boussier* + +* Fix and add protections for XSS in `ActionView::Helpers` and `ERB::Util`. + + Add the method `ERB::Util.xml_name_escape` to escape dangerous characters + in names of tags and names of attributes, following the specification of XML. + + *Álvaro Martín Fraguas* + +* Respect `ActiveSupport::Logger.new`'s `:formatter` keyword argument + + The stdlib `Logger::new` allows passing a `:formatter` keyword argument to + set the logger's formatter. Previously `ActiveSupport::Logger.new` ignored + that argument by always setting the formatter to an instance of + `ActiveSupport::Logger::SimpleFormatter`. + + *Steven Harman* + +* Deprecate preserving the pre-Ruby 2.4 behavior of `to_time` + + With Ruby 2.4+ the default for +to_time+ changed from converting to the + local system time to preserving the offset of the receiver. At the time Rails + supported older versions of Ruby so a compatibility layer was added to assist + in the migration process. From Rails 5.0 new applications have defaulted to + the Ruby 2.4+ behavior and since Rails 7.0 now only supports Ruby 2.7+ + this compatibility layer can be safely removed. + + To minimize any noise generated the deprecation warning only appears when the + setting is configured to `false` as that is the only scenario where the + removal of the compatibility layer has any effect. + + *Andrew White* + +* `Pathname.blank?` only returns true for `Pathname.new("")` + + Previously it would end up calling `Pathname#empty?` which returned true + if the path existed and was an empty directory or file. + + That behavior was unlikely to be expected. + + *Jean Boussier* + +* Deprecate `Notification::Event`'s `#children` and `#parent_of?` + +* Change default serialization format of `MessageEncryptor` from `Marshal` to `JSON` for Rails 7.1. + + Existing apps are provided with an upgrade path to migrate to `JSON` as described in `guides/source/upgrading_ruby_on_rails.md` + + *Zack Deveau* and *Martin Gingras* + +* Add `ActiveSupport::TestCase#stub_const` to stub a constant for the duration of a yield. + + *DHH* + +* Fix `ActiveSupport::EncryptedConfiguration` to be compatible with Psych 4 + + *Stephen Sugden* + +* Improve `File.atomic_write` error handling + +* Fix `Class#descendants` and `DescendantsTracker#descendants` compatibility with Ruby 3.1. + + [The native `Class#descendants` was reverted prior to Ruby 3.1 release](https://bugs.ruby-lang.org/issues/14394#note-33), + but `Class#subclasses` was kept, breaking the feature detection. + + *Jean Boussier* + +Please check [7-0-stable](https://github.com/rails/rails/blob/7-0-stable/activesupport/CHANGELOG.md) for previous changes. diff --git a/tools/rail_inspector/test/fixtures/railties_06e9fbd.md b/tools/rail_inspector/test/fixtures/railties_06e9fbd.md new file mode 100644 index 00000000000..b85eb486caf --- /dev/null +++ b/tools/rail_inspector/test/fixtures/railties_06e9fbd.md @@ -0,0 +1,215 @@ +* In-app custom credentials templates are now supported. When a credentials + file does not exist, `rails credentials:edit` will now try to use + `lib/templates/rails/credentials/credentials.yml.tt` to generate the + credentials file, before falling back to the default template. + + This allows e.g. an open-source Rails app (which would not include encrypted + credentials files in its repo) to include a credentials template, so that + users who install the app will get a custom pre-filled credentials file when + they run `rails credentials:edit`. + + *Jonathan Hefner* + +* Except for `dev` and `test` environments, newly generated per-environment + credentials files (e.g. `config/credentials/production.yml.enc`) now include + a `secret_key_base` for convenience, just as `config/credentials.yml.enc` + does. + + *Jonathan Hefner* + +* `--no-*` options now work with the app generator's `--minimal` option, and + are both comprehensive and precise. For example: + + ```console + $ rails new my_cool_app --minimal + Based on the specified options, the following options will also be activated: + + --skip-active-job [due to --minimal] + --skip-action-mailer [due to --skip-active-job, --minimal] + --skip-active-storage [due to --skip-active-job, --minimal] + --skip-action-mailbox [due to --skip-active-storage, --minimal] + --skip-action-text [due to --skip-active-storage, --minimal] + --skip-javascript [due to --minimal] + --skip-hotwire [due to --skip-javascript, --minimal] + --skip-action-cable [due to --minimal] + --skip-bootsnap [due to --minimal] + --skip-dev-gems [due to --minimal] + --skip-system-test [due to --minimal] + + ... + + $ rails new my_cool_app --minimal --no-skip-active-storage + Based on the specified options, the following options will also be activated: + + --skip-action-mailer [due to --minimal] + --skip-action-mailbox [due to --minimal] + --skip-action-text [due to --minimal] + --skip-javascript [due to --minimal] + --skip-hotwire [due to --skip-javascript, --minimal] + --skip-action-cable [due to --minimal] + --skip-bootsnap [due to --minimal] + --skip-dev-gems [due to --minimal] + --skip-system-test [due to --minimal] + + ... + ``` + + *Brad Trick* and *Jonathan Hefner* + +* Add `--skip-dev-gems` option to app generator to skip adding development + gems (like `web-console`) to the Gemfile. + + *Brad Trick* + +* Skip Active Storage and Action Mailer if Active Job is skipped. + + *Étienne Barrié* + +* Correctly check if frameworks are disabled when running app:update. + + *Étienne Barrié* and *Paulo Barros* + +* Delegate model generator description to orm hooked generator. + + *Gannon McGibbon* + +* Execute `rails runner` scripts inside the executor. + + Enables error reporting, query cache, etc. + + *Jean Boussier* + +* Avoid booting in development then test for test tasks. + + Running one of the rails test subtasks (e.g. test:system, test:models) would + go through Rake and cause the app to be booted twice. Now all the test:* + subtasks are defined as Thor tasks and directly load the test environment. + + *Étienne Barrié* + +* Deprecate `Rails::Generators::Testing::Behaviour` in favor of `Rails::Generators::Testing::Behavior`. + + *Gannon McGibbon* + +* Allow configuration of logger size for local and test environments + + `config.log_file_size` + + Defaults to `100` megabytes. + + *Bernie Chiu* + +* Enroll new apps in decrypted diffs of credentials by default. This behavior + can be opted out of with the app generator's `--skip-decrypted-diffs` flag. + + *Jonathan Hefner* + +* Support declarative-style test name filters with `bin/rails test`. + + This makes it possible to run a declarative-style test such as: + + ```ruby + class MyTest < ActiveSupport::TestCase + test "does something" do + # ... + end + end + ``` + + Using its declared name: + + ```bash + $ bin/rails test test/my_test.rb -n "does something" + ``` + + Instead of having to specify its expanded method name: + + ```bash + $ bin/rails test test/my_test.rb -n test_does_something + ``` + + *Jonathan Hefner* + +* Add `--js` and `--skip-javascript` options to `rails new` + + `--js` alias to `rails new --javascript ...` + + Same as `-j`, e.g. `rails new --js esbuild ...` + + `--skip-js` alias to `rails new --skip-javascript ...` + + Same as `-J`, e.g. `rails new --skip-js ...` + + *Dorian Marié* + +* Allow relative paths with leading dot slash to be passed to `rails test`. + + Fix `rails test ./test/model/post_test.rb` to run a single test file. + + *Shouichi Kamiya* and *oljfte* + +* Deprecate `config.enable_dependency_loading`. This flag addressed a limitation of the `classic` autoloader and has no effect nowadays. To fix this deprecation, please just delete the reference. + + *Xavier Noria* + +* Define `config.enable_reloading` to be `!config.cache_classes` for a more intuitive name. While `config.enable_reloading` and `config.reloading_enabled?` are preferred from now on, `config.cache_classes` is supported for backwards compatibility. + + *Xavier Noria* + +* Add JavaScript dependencies installation on bin/setup + + Add `yarn install` to bin/setup when using esbuild, webpack, or rollout. + + *Carlos Ribeiro* + +* Use `controller_class_path` in `Rails::Generators::NamedBase#route_url` + + The `route_url` method now returns the correct path when generating + a namespaced controller with a top-level model using `--model-name`. + + Previously, when running this command: + + ``` sh + bin/rails generate scaffold_controller Admin/Post --model-name Post + ``` + + the comments above the controller action would look like: + + ``` ruby + # GET /posts + def index + @posts = Post.all + end + ``` + + afterwards, they now look like this: + + ``` ruby + # GET /admin/posts + def index + @posts = Post.all + end + ``` + + Fixes #44662. + + *Andrew White* + +* No longer add autoloaded paths to `$LOAD_PATH`. + + This means it won't be possible to load them with a manual `require` call, the class or module can be referenced instead. + + Reducing the size of `$LOAD_PATH` speed-up `require` calls for apps not using `bootsnap`, and reduce the + size of the `bootsnap` cache for the others. + + *Jean Boussier* + +* Remove default `X-Download-Options` header + + This header is currently only used by Internet Explorer which + will be discontinued in 2022 and since Rails 7 does not fully + support Internet Explorer this header should not be a default one. + + *Harun Sabljaković* + +Please check [7-0-stable](https://github.com/rails/rails/blob/7-0-stable/railties/CHANGELOG.md) for previous changes. diff --git a/tools/rail_inspector/test/rail_inspector/changelog_test.rb b/tools/rail_inspector/test/rail_inspector/changelog_test.rb new file mode 100644 index 00000000000..1458709d44f --- /dev/null +++ b/tools/rail_inspector/test/rail_inspector/changelog_test.rb @@ -0,0 +1,147 @@ +# frozen_string_literal: true + +require "rail_inspector/changelog" +require "test/test_helpers/changelog_fixtures" + +class TestChangelog < Minitest::Test + include ChangelogFixtures + + def test_parses_changelog_file + @changelog = changelog_fixture("railties_06e9fbd.md") + + assert_equal 21, entries.length + end + + def test_entries_without_author_are_invalid + @changelog = changelog_fixture("active_support_9f0b8eb.md") + + assert_equal 2, offenses.length + end + + def test_parses_with_extra_newlines + @changelog = changelog_fixture("action_mailbox_83d85b2.md") + + assert_equal 0, entries.length + end + + def test_entries_with_trailing_whitespace_are_invalid + @changelog = changelog_fixture("active_record_936a862.md") + + assert_equal 16, offenses.length + end + + def test_entries_without_four_leading_spaces + @changelog = changelog_fixture("active_record_238432d.md") + + assert_equal 5, offenses.length + end + + def test_entries_with_incorrectly_indented_header + @changelog = changelog_fixture("active_record_51852d2.md") + + assert_equal 1, offenses.length + end + + def test_header_ending_with_star_not_treated_as_author + @changelog = changelog_fixture("action_pack_69d504.md") + + assert_equal 0, offenses.length + end + + def test_validate_authors + assert_offense(<<~CHANGELOG) + * Fix issue in CHANGELOG linting + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ CHANGELOG entry is missing authors. + CHANGELOG + end + + def test_validate_leading_whitespace_for_header + assert_offense(<<~CHANGELOG) + * Fix leading whitespace in CHANGELOG + ^^^^ CHANGELOG header must start with '*' and 3 spaces + + *Hartley McGuire* + CHANGELOG + + assert_offense(<<~CHANGELOG) + * Fix leading whitespace in CHANGELOG + ^^^^ CHANGELOG header must start with '*' and 3 spaces + + *Hartley McGuire* + CHANGELOG + end + + def test_validate_leading_whitespace_for_body + assert_offense(<<~CHANGELOG) + * Fix leading whitespace in CHANGELOG + + *Hartley McGuire* + ^^^^ CHANGELOG line must be indented 4 spaces + CHANGELOG + end + + def test_validate_trailing_whitespace + assert_offense(<<~CHANGELOG) + * Fix trailing whitespace in CHANGELOG#{' '} + ^ Trailing whitespace detected. + + *Hartley McGuire* + CHANGELOG + + assert_offense(<<~CHANGELOG) + * Fix trailing whitespace in CHANGELOG + #{' '} + ^ Trailing whitespace detected. + *Hartley McGuire* + CHANGELOG + end + + private + def entries + @changelog.entries + end + + def offenses + entries.flat_map(&:offenses) + end + + ANNOTATION_PATTERN = /\s*\^+ / + + def assert_offense(source) + lines = [] + annotation = nil + + source.each_line(chomp: true) do |line| + if ANNOTATION_PATTERN.match?(line) + annotation = [lines.length, line] + else + lines << line + end + end + + entry = RailInspector::Changelog::Entry.new(lines, 1) + + assert_equal 1, + entry.offenses.length, + "Entry has the wrong number of offenses" + offense = entry.offenses.first + + assert_equal annotation[0], + offense.line_number, + "Offense has incorrect line number" + assert_equal lines[annotation[0] - 1], + offense.line, + "Offense has incorrect line" + + annotation_message = annotation[1].gsub(ANNOTATION_PATTERN, "") + assert_equal annotation_message, + offense.message, + "Offense has incorrect message" + + annotation_start = annotation[1].index("^") + 1 + annotation_end = annotation[1].rindex("^") + 1 + assert_equal annotation_start..annotation_end, + offense.range, + "Offense has incorrect range" + end +end diff --git a/tools/rail_inspector/test/rail_inspector/visitor/framework_default_test.rb b/tools/rail_inspector/test/rail_inspector/visitor/framework_default_test.rb new file mode 100644 index 00000000000..b1497d69f08 --- /dev/null +++ b/tools/rail_inspector/test/rail_inspector/visitor/framework_default_test.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +require "rail_inspector/visitor/framework_default" + +class FrameworkDefaultTest < Minitest::Test + def test_smoke + config = config_for_defaults <<~RUBY + case target_version.to_s + when "5.0" + ActiveSupport.to_time_preserves_timezone = true + + if respond_to?(:active_record) + active_record.belongs_to_required_by_default = true + end + + self.ssl_options = { hsts: { subdomains: true } } + when "5.1" + load_defaults "5.0" + + if respond_to?(:assets) + assets.unknown_asset_fallback = false + end + else + raise + end + RUBY + + ["5.0", "5.1"].each { |k| assert_includes(config, k) } + + assert_equal config["5.0"]["ActiveSupport.to_time_preserves_timezone"], "true" + assert_equal config["5.0"]["active_record.belongs_to_required_by_default"], "true" + assert_equal config["5.0"]["self.ssl_options"], "{ hsts: { subdomains: true } }" + assert_equal config["5.1"]["assets.unknown_asset_fallback"], "false" + end + + def test_config_wrapped_in_condition + config = config_for_defaults <<~RUBY + case target_version.to_s + when "7.1" + if Rails.env.local? + self.log_file_size = 100 * 1024 * 1024 + end + end + RUBY + + assert_includes config, "7.1" + assert_equal config["7.1"]["self.log_file_size"], "100 * 1024 * 1024" + end + + def test_condition_inside_framework + config = config_for_defaults <<~RUBY + case target_version.to_s + when "7.1" + if respond_to?(:action_view) + if Rails::HTML::Sanitizer.html5_support? + action_view.sanitizer_vendor = Rails::HTML5::Sanitizer + end + end + end + RUBY + + assert_includes config, "7.1" + assert_equal config["7.1"]["action_view.sanitizer_vendor"], "Rails::HTML5::Sanitizer" + end + + def test_nested_frameworks_raise_when_strict + original_env, ENV["STRICT"] = ENV["STRICT"], "true" + + assert_raises do + config_for_defaults <<~RUBY + case target_version.to_s + when "7.1" + if respond_to?(:action_view) + if respond_to?(:active_record) + end + end + end + RUBY + end + ensure + ENV["STRICT"] = original_env + end + + private + def wrapped_defaults(defaults) + <<~RUBY + class Configuration + def load_defaults(target_version) + #{defaults} + end + end + RUBY + end + + def config_for_defaults(defaults) + full_class = wrapped_defaults(defaults) + parsed = SyntaxTree.parse(full_class) + visitor.visit(parsed) + visitor.config_map + end + + def visitor + @visitor ||= RailInspector::Visitor::FrameworkDefault.new + end +end diff --git a/tools/rail_inspector/test/rail_inspector/visitor/hash_to_string_test.rb b/tools/rail_inspector/test/rail_inspector/visitor/hash_to_string_test.rb new file mode 100644 index 00000000000..8cf20601aaf --- /dev/null +++ b/tools/rail_inspector/test/rail_inspector/visitor/hash_to_string_test.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require "syntax_tree" + +require "rail_inspector/visitor/hash_to_string" + +class HashToStringTest < Minitest::Test + def test_basic_hash_to_s + basic_hash = "{ a: 1 }" + + assert_equal basic_hash, string_for(basic_hash) + end + + def test_nested_hash_to_s + nested_hash = "{ hsts: { subdomains: true } }" + + assert_equal nested_hash, string_for(nested_hash) + end + + def test_string_keys_to_s + string_keys = + '{ "X-Frame-Options" => "SAMEORIGIN", "X-XSS-Protection" => "0", "X-Content-Type-Options" => "nosniff", "X-Permitted-Cross-Domain-Policies" => "none", "Referrer-Policy" => "strict-origin-when-cross-origin" }' + + assert_equal string_keys, string_for(string_keys) + end + + private + def string_for(hash_as_string) + ast = SyntaxTree.parse(hash_as_string) + visitor = RailInspector::Visitor::HashToString.new + visitor.visit(ast) + visitor.to_s + end +end diff --git a/tools/rail_inspector/test/test_helpers/changelog_fixtures.rb b/tools/rail_inspector/test/test_helpers/changelog_fixtures.rb new file mode 100644 index 00000000000..5c93c670dd5 --- /dev/null +++ b/tools/rail_inspector/test/test_helpers/changelog_fixtures.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require "pathname" +require "rail_inspector/changelog" + +module ChangelogFixtures + def changelog_fixture(name) + path = Pathname.new(File.expand_path("../fixtures/#{name}", __dir__)) + + raise ArgumentError, "#{name} fixture not found" unless path.exist? + + RailInspector::Changelog.new(path, path.read) + end +end diff --git a/tools/railspect b/tools/railspect new file mode 100755 index 00000000000..99b6f7e2165 --- /dev/null +++ b/tools/railspect @@ -0,0 +1,7 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +require "bundler/setup" +require_relative "rail_inspector/lib/rail_inspector/cli" + +RailInspector::Cli.start