From 5a9ef5c5a69ac6c761cd860e89a05645d6efbae0 Mon Sep 17 00:00:00 2001 From: Hartley McGuire Date: Thu, 26 Dec 2024 19:59:36 -0500 Subject: [PATCH] [rail_inspector] Prism Visitor::FrameworkDefault Also apply tweaks to Configuration guide --- Gemfile | 4 - Gemfile.lock | 4 - guides/source/configuring.md | 2 +- .../lib/rail_inspector/configuring.rb | 2 +- .../check/general_configuration.rb | 2 +- .../visitor/framework_default.rb | 166 +++++++++--------- .../visitor/multiline_to_string.rb | 27 --- tools/rail_inspector/rail_inspector.gemspec | 2 +- .../visitor/framework_default_test.rb | 34 +++- 9 files changed, 116 insertions(+), 127 deletions(-) delete mode 100644 tools/rail_inspector/lib/rail_inspector/visitor/multiline_to_string.rb diff --git a/Gemfile b/Gemfile index 8c295b79f890d..e85d282a172de 100644 --- a/Gemfile +++ b/Gemfile @@ -45,10 +45,6 @@ gem "uri", ">= 0.13.1", require: false gem "prism" -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 e05aa6e85003f..1d5300112bdf2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -423,7 +423,6 @@ GEM racc path_expander (1.1.3) pg (1.5.8) - prettier_print (1.2.1) prism (1.2.0) propshaft (1.1.0) actionpack (>= 7.0.0) @@ -617,8 +616,6 @@ GEM stringio (3.1.1) sucker_punch (3.2.0) concurrent-ruby (~> 1.0) - syntax_tree (6.1.1) - prettier_print (>= 1.2.0) tailwindcss-rails (3.0.0) railties (>= 7.0.0) tailwindcss-ruby @@ -743,7 +740,6 @@ DEPENDENCIES stackprof stimulus-rails sucker_punch - syntax_tree (= 6.1.1) tailwindcss-rails terser (>= 1.1.4) thruster diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 29edaa28b46cd..6fae0fd3fb66b 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -72,7 +72,7 @@ Below are the default values associated with each target version. In cases of co - [`config.active_record.postgresql_adapter_decode_dates`](#config-active-record-postgresql-adapter-decode-dates): `true` - [`config.active_record.validate_migration_timestamps`](#config-active-record-validate-migration-timestamps): `true` -- [`config.active_storage.web_image_content_types`](#config-active-storage-web-image-content-types): `%w[image/png image/jpeg image/gif image/webp]` +- [`config.active_storage.web_image_content_types`](#config-active-storage-web-image-content-types): `%w( image/png image/jpeg image/gif image/webp )` - [`config.yjit`](#config-yjit): `true` #### Default Values for Target Version 7.1 diff --git a/tools/rail_inspector/lib/rail_inspector/configuring.rb b/tools/rail_inspector/lib/rail_inspector/configuring.rb index e5e36a62c28a8..679071219b6ad 100644 --- a/tools/rail_inspector/lib/rail_inspector/configuring.rb +++ b/tools/rail_inspector/lib/rail_inspector/configuring.rb @@ -12,7 +12,7 @@ def initialize end def call(path) - @cache[path] ||= SyntaxTree.parse(SyntaxTree.read(path)) + @cache[path] ||= Prism.parse_file(path.to_s).value 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 index aa65271ba5bec..95a2b05bf8f2f 100644 --- a/tools/rail_inspector/lib/rail_inspector/configuring/check/general_configuration.rb +++ b/tools/rail_inspector/lib/rail_inspector/configuring/check/general_configuration.rb @@ -14,7 +14,7 @@ def initialize(checker) def call visitor = Visitor::Attribute.new visitor.visit(app_config_tree) - visitor.attribute_map[APP_CONFIG_CONST]["attr_accessor"] + visitor.attribute_map[APP_CONFIG_CONST][:attr_accessor] end private diff --git a/tools/rail_inspector/lib/rail_inspector/visitor/framework_default.rb b/tools/rail_inspector/lib/rail_inspector/visitor/framework_default.rb index 8761940c50a06..9fe61a6b4bea9 100644 --- a/tools/rail_inspector/lib/rail_inspector/visitor/framework_default.rb +++ b/tools/rail_inspector/lib/rail_inspector/visitor/framework_default.rb @@ -1,26 +1,12 @@ # frozen_string_literal: true -require "syntax_tree" +require "prism" 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 @@ -28,22 +14,23 @@ def initialize end def visit(node) - case_node, *others = TargetVersionCaseFinder.scan(node).to_a - raise "#{others.length} other cases?" unless others.empty? + target_version_case = node.breadth_first_search do |n| + n in Prism::CaseNode[ + predicate: Prism::CallNode[receiver: Prism::LocalVariableReadNode[name: :target_version]] + ] + end - visit_when(case_node.consequent) + target_version_case.conditions.each { |cond| visit_when(cond) } end private def visit_when(node) - version = node.arguments.parts[0].parts[0].value - - config_map[version] = VersionedConfig.new.config_for(node.statements) + version = node.conditions[0].unescaped - visit_when(node.consequent) if node.consequent.is_a? SyntaxTree::When + config_map[version] = VersionedConfig.new.tap { |v| v.visit(node.statements) }.configs end - class VersionedConfig < SyntaxTree::Visitor + class VersionedConfig < Prism::Visitor attr_reader :configs def initialize @@ -51,87 +38,92 @@ def initialize @framework_stack = [] end - def config_for(node) - visit(node) - @configs - end + def visit_if_node(node) + unless new_framework = respond_to_framework?(node.predicate) + return visit_child_nodes(node) + 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 - if ENV["STRICT"] && current_framework - raise "Potentially nested framework? Current: '#{current_framework}', found: '#{new_framework}'" - end + @framework_stack << new_framework + visit_child_nodes(node) + @framework_stack.pop + end + + def visit_call_node(node) + name = node.name.to_s - @framework_stack << new_framework - super - @framework_stack.pop + unless name.end_with? "=" + return super 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 + handle_assignment(node, name[...-1], node.arguments.arguments[0]) + end + + def visit_call_or_write_node(node) + name = node.write_name.to_s + + unless name.end_with? "=" + return super end - def visit_opassign(node) - if node.operator.name == :"||=" - visit_assign(node) - else - super + handle_assignment(node, node.read_name.to_s, node.value) + end + + def handle_assignment(node, name, value) + prefix = case node.receiver + in Prism::ConstantReadNode[name: constant_name] + constant_name + in Prism::SelfNode + "self" + in Prism::CallNode[receiver: nil, name: framework] + framework_string = framework.to_s + + unless current_framework == framework_string + raise "expected: #{current_framework}, actual: #{framework_string}" end + + framework_string + else + node.receiver.location.slice 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}" + target = "#{prefix}.#{name}" + + string_value = case value + in Prism::ConstantPathNode + value.full_name + in Prism::HashNode + HashToString.new.tap { |v| v.visit(value) }.to_s + in Prism::InterpolatedStringNode + "\"#{value.parts.map(&:content).join("")}\"" + in Prism::FalseNode + "false" + in Prism::TrueNode + "true" + else + value.location.slice end + @configs[target] = string_value + end + + private def current_framework @framework_stack.last end def respond_to_framework?(node) - if node in SyntaxTree::CallNode[ - receiver: nil, - message: SyntaxTree::Ident[value: "respond_to?"], - arguments: SyntaxTree::ArgParen[ - arguments: SyntaxTree::Args[ - parts: [ - SyntaxTree::SymbolLiteral[ - value: SyntaxTree::Ident[value: new_framework] - ] - ] - ] - ] - ] + if node in Prism::CallNode[ + name: :respond_to?, + arguments: Prism::ArgumentsNode[ + arguments: [ + Prism::SymbolNode[unescaped: new_framework] + ] + ] + ] new_framework 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 deleted file mode 100644 index 64aa0d60e97ce..0000000000000 --- a/tools/rail_inspector/lib/rail_inspector/visitor/multiline_to_string.rb +++ /dev/null @@ -1,27 +0,0 @@ -# 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 index 2bad3c9ade918..329c8741ac8c9 100644 --- a/tools/rail_inspector/rail_inspector.gemspec +++ b/tools/rail_inspector/rail_inspector.gemspec @@ -29,6 +29,6 @@ Gem::Specification.new do |spec| spec.require_paths = ["lib"] # Uncomment to register a new dependency of your gem - spec.add_dependency "syntax_tree", "6.1.1" + spec.add_dependency "prism", "~> 1.2" spec.add_dependency "thor", "~> 1.0" 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 index f38b7ca3b1610..ff86edba9cacf 100644 --- a/tools/rail_inspector/test/rail_inspector/visitor/framework_default_test.rb +++ b/tools/rail_inspector/test/rail_inspector/visitor/framework_default_test.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "test_helper" require "rail_inspector/visitor/framework_default" class FrameworkDefaultTest < Minitest::Test @@ -81,6 +82,37 @@ def test_nested_frameworks_raise_when_strict ENV["STRICT"] = original_env end + def test_multiline_strings + config = config_for_defaults <<~RUBY + case target_version.to_s + when "7.0" + if respond_to?(:active_storage) + active_storage.video_preview_arguments = + "-vf 'select=eq(n\\,0)+eq(key\\,1)+gt(scene\\,0.015),loop=loop=-1:size=2,trim=start_frame=1'" \ + " -frames:v 1 -f image2" + end + end + RUBY + + assert_includes config, "7.0" + assert_equal( + "\"-vf 'select=eq(n\\,0)+eq(key\\,1)+gt(scene\\,0.015),loop=loop=-1:size=2,trim=start_frame=1' -frames:v 1 -f image2\"", + config["7.0"]["active_storage.video_preview_arguments"], + ) + end + + def test_inline_condition + config = config_for_defaults <<~RUBY + case target_version.to_s + when "8.0" + Regexp.timeout ||= 1 if Regexp.respond_to?(:timeout=) + end + RUBY + + assert_includes config, "8.0" + assert_equal("1", config["8.0"]["Regexp.timeout"]) + end + private def wrapped_defaults(defaults) <<~RUBY @@ -94,7 +126,7 @@ def load_defaults(target_version) def config_for_defaults(defaults) full_class = wrapped_defaults(defaults) - parsed = SyntaxTree.parse(full_class) + parsed = Prism.parse(full_class).value visitor.visit(parsed) visitor.config_map end