diff --git a/.ruby-version b/.ruby-version index 37c2961c2..a4dd9dba4 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.7.2 +2.7.4 diff --git a/.standard.yml b/.standard.yml index 2257d7c29..5b43beacc 100644 --- a/.standard.yml +++ b/.standard.yml @@ -1,2 +1,4 @@ ignore: - "templates/partials/db_optimizations_configuration.rb" + - "templates/partials/pull_requests_config.rb" + - "templates/partials/email_smtp.rb" diff --git a/lib/suspenders.rb b/lib/suspenders.rb index 81a71a14b..36cc2824f 100644 --- a/lib/suspenders.rb +++ b/lib/suspenders.rb @@ -27,5 +27,6 @@ require "suspenders/generators/production/single_redirect" require "suspenders/generators/staging/pull_requests_generator" require "suspenders/actions" +require "suspenders/actions/strip_comments_action" require "suspenders/adapters/heroku" require "suspenders/app_builder" diff --git a/lib/suspenders/actions.rb b/lib/suspenders/actions.rb index fafa90aa8..f45938521 100644 --- a/lib/suspenders/actions.rb +++ b/lib/suspenders/actions.rb @@ -6,7 +6,7 @@ def replace_in_file(relative_path, find, replace) unless contents.gsub!(find, replace) raise "#{find.inspect} not found in #{relative_path}" end - File.open(path, "w") { |file| file.write(contents) } + File.write(path, contents) end def action_mailer_host(rails_env, host) diff --git a/lib/suspenders/actions/strip_comments_action.rb b/lib/suspenders/actions/strip_comments_action.rb new file mode 100644 index 000000000..3abb37fd6 --- /dev/null +++ b/lib/suspenders/actions/strip_comments_action.rb @@ -0,0 +1,254 @@ +require "parser/current" + +module Suspenders + module Actions + class StripCommentsAction + class << self + def call(source) + parser = Parser::CurrentRuby.new + + source + .then { |s| strip_comments(s, parser) } + .then { |s| strip_trailing_whitespace(s) } + .then { |s| strip_dup_newlines(s) } + .then { |s| strip_leading_scope_newlines(s, parser) } + end + + private + + def strip_comments(source, parser) + StripComments.call(source, parser.reset) + end + + def strip_trailing_whitespace(source) + source.gsub(/[[:blank:]]+$/, "") + end + + def strip_dup_newlines(source) + source.gsub(/\n{2,}/, "\n\n") + end + + def strip_leading_scope_newlines(source, parser) + StripLeadingScopeNewlines.call(source, parser.reset) + end + end + + # Strips full-line and inline comments from a buffer but does + # not remove whitespaces or newlines after the fact. Example + # input: + # + # MyGem.application.configure do |config| + # # Full-line comment + # config.option1 = :value # Inline comment + # end + # + # The output is: + # + # MyGem.application.configure do |config| + # + # config.option1 = :value + # end + class StripComments + class << self + def call(source, parser) + buffer = Parser::Source::Buffer.new(nil, source: source) + rewriter = Parser::Source::TreeRewriter.new(buffer) + + _, comments = parser.parse_with_comments(buffer) + + comments.each do |comment| + strip_comment(comment, buffer, rewriter) + end + + rewriter.process + end + + private + + def strip_comment(comment, buffer, rewriter) + expr = comment.location.expression + + if full_line_comment?(expr) + expr = full_line_comment_expr(expr, buffer) + end + + rewriter.remove(expr) + end + + def full_line_comment_expr(expr, buffer) + pos = BackwardStringScanner.beginning_of_line_pos(expr, expr.begin_pos) + + Parser::Source::Range.new(buffer, pos, expr.end_pos + 1) + end + + def full_line_comment?(expr) + expr.source == expr.source_line.lstrip + end + end + end + + # A tiny, non-stateful backward string scanner somewhat inspired + # by Ruby's StringScanner. Ruby's StringScanner is unable to + # seek backward on a string. + class BackwardStringScanner + def self.beginning_of_line_pos(expr, initial_pos) + skip_before(expr, initial_pos) { |char| char == "\n" } + end + + def self.skip_before(expr, initial_pos, &block) + skip_until(expr, initial_pos, -1, &block) + end + + def self.skip_until(expr, initial_pos, lookup_inc = 0) + pos = initial_pos + + loop do + break if pos.zero? + char = expr.source_buffer.source[pos + lookup_inc] + break if yield(char) + pos -= 1 + end + + pos + end + end + + # The intent of this class is purely aesthetic: remove leading + # newlines inside of code scopes like blocks and begin/end. + # Example input: + # + # module MyGem + # + # MyGem.application.configure do |config| + # + # config.option1 = true + # + # config.option2 = false + # end + # end + # + # The output is: + # + # module MyGem + # MyGem.application.configure do |config| + # config.option1 = true + # + # config.option2 = false + # end + # end + class StripLeadingScopeNewlines + def self.call(source, parser) + buffer = Parser::Source::Buffer.new(nil, source: source) + ast = parser.parse(buffer) + + LeadingNewlineStripRewriter.new.rewrite(buffer, ast).lstrip + end + + class LeadingNewlineStripRewriter < Parser::TreeRewriter + def on_module(node) + strip_newline_before(node.children[1]) + strip_newline_after(node.children.last) + + super + end + + def on_class(node) + strip_newline_before(node.children[2]) + strip_newline_after(node.children.last) + + super + end + + def on_begin(node) + handle_begin(node) + + super + end + + def on_kwbegin(node) + strip_newline_before(node.children[0]) + strip_newline_after(node.children.last) + + handle_begin(node) + + super + end + + def on_block(node) + strip_newline_before(node.children[2]) + strip_newline_after(node.children.last) + + super + end + + private + + def handle_begin(node) + strip_blank_lines_between_setter_calls(node.children) + + node.children.each do |child_node| + send("on_#{child_node.type}", child_node) + end + end + + def strip_blank_lines_between_setter_calls(children) + pairs = children.each_cons(2).to_a + + pairs.each do |(node_before, node_after)| + if setter_call?(node_before) && setter_call?(node_after) + strip_newline_before(node_after) + end + end + end + + def setter_call?(node) + node.children[1].to_s.end_with?("=") + end + + def strip_newline_before(node) + return if node.nil? + + expr = node.location.expression + end_pos = find_end_pos(expr, expr.begin_pos) + begin_pos = find_begin_pos(expr, end_pos) + + strip_source_range(expr, begin_pos, end_pos) + end + + def strip_newline_after(node) + return if node.nil? + + expr = node.location.expression + source = expr.source_buffer.source + + if source[expr.end_pos + 1] == "\n" + strip_source_range(expr, expr.end_pos, expr.end_pos + 1) + end + end + + def find_end_pos(expr, begin_pos) + BackwardStringScanner.skip_until(expr, begin_pos) do |char| + char == "\n" + end + end + + def find_begin_pos(expr, end_pos) + BackwardStringScanner.skip_before(expr, end_pos) do |char| + char != "\n" && char != " " + end + end + + def strip_source_range(expr, begin_pos, end_pos) + remove( + Parser::Source::Range.new( + expr.source_buffer, + begin_pos, + end_pos + ) + ) + end + end + end + end + end +end diff --git a/lib/suspenders/app_builder.rb b/lib/suspenders/app_builder.rb index 64e83070f..26b2cf699 100644 --- a/lib/suspenders/app_builder.rb +++ b/lib/suspenders/app_builder.rb @@ -205,15 +205,10 @@ def remove_config_comment_lines ] config_files.each do |config_file| - path = File.join(destination_root, "config/#{config_file}") + path = Pathname(destination_root).join("config", config_file) + source = Actions::StripCommentsAction.call(path.read) - accepted_content = File.readlines(path).reject { |line| - line =~ /^\s*#.*$/ || line =~ /^$\n/ - } - - File.open(path, "w") do |file| - accepted_content.each { |line| file.puts line } - end + path.write(source) end end diff --git a/lib/suspenders/generators/production/email_generator.rb b/lib/suspenders/generators/production/email_generator.rb index b15312616..194eb9373 100644 --- a/lib/suspenders/generators/production/email_generator.rb +++ b/lib/suspenders/generators/production/email_generator.rb @@ -7,14 +7,14 @@ def smtp_configuration copy_file "smtp.rb", "config/smtp.rb" prepend_file "config/environments/production.rb", - %{require Rails.root.join("config/smtp")\n} + %{require Rails.root.join("config/smtp")\n\n} end def use_smtp inject_template_into_file( "config/environments/production.rb", "partials/email_smtp.rb", - after: "config.action_mailer.perform_caching = false" + after: "config.action_mailer.perform_caching = false\n" ) end diff --git a/spec/support/file_operations.rb b/spec/support/file_operations.rb index bc223f3bc..45aa22e7f 100644 --- a/spec/support/file_operations.rb +++ b/spec/support/file_operations.rb @@ -1,6 +1,10 @@ module FileOperations module_function + def read_file(file) + TestPaths.app_path.join(file).read + end + def touch_file(file) path = app_path.join(file) path.join("..").mkpath diff --git a/spec/suspenders/actions/strip_comments_action_spec.rb b/spec/suspenders/actions/strip_comments_action_spec.rb new file mode 100644 index 000000000..0bca11dff --- /dev/null +++ b/spec/suspenders/actions/strip_comments_action_spec.rb @@ -0,0 +1,489 @@ +require "spec_helper" +require "suspenders/actions/strip_comments_action" + +RSpec.describe Suspenders::Actions::StripCommentsAction do + describe ".call" do + it "removes a comment at indentation 0" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + # Comment 1 + Rails.application.configure do + config.force_ssl = true + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + Rails.application.configure do + config.force_ssl = true + end + SOURCE + end + + it "removes a comment at indentation 2" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + Rails.application.configure do + # A comment + config.force_ssl = true + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + Rails.application.configure do + config.force_ssl = true + end + SOURCE + end + + it "removes two comments at indentation 2" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + Rails.application.configure do + # Comment 1 + # Comment 2 + config.force_ssl = true + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + Rails.application.configure do + config.force_ssl = true + end + SOURCE + end + + it "removes inline comments" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + Rails.application.configure do + config.force_ssl = true # A comment here + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + Rails.application.configure do + config.force_ssl = true + end + SOURCE + end + + it "does not remove non-comments that could be confused as comments" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + Rails.application.configure do + str = <<~STR + # Not a comment + STR + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + Rails.application.configure do + str = <<~STR + # Not a comment + STR + end + SOURCE + end + + it "preserves the newline at the end of the file" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + # Comment 1 + Rails.application.configure do + config.force_ssl = true + end + SOURCE + + expect(source).to eq(<<~SOURCE) + Rails.application.configure do + config.force_ssl = true + end + SOURCE + end + + it "removes comments that delineate blocks of code" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + # Comment 1 + Rails.application.configure do + # Comment 1 + config.force_ssl = true + config.cache_store = :null_store + + # Comment 2 + config.eager_load = true + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + Rails.application.configure do + config.force_ssl = true + config.cache_store = :null_store + config.eager_load = true + end + SOURCE + end + + it "removes blank lines with whitespace" do + source = <<~SOURCE + # One + config.n = true + #{" " * 10} + # Two + config.t = true + SOURCE + + source = Suspenders::Actions::StripCommentsAction.call(source) + + expect(source.chomp).to eq(<<~SOURCE.chomp) + config.n = true + config.t = true + SOURCE + end + + it "removes trailing whitespace" do + source = <<~SOURCE + method do + config.n = true#{" " * 5} + config.t = true + end + SOURCE + + source = Suspenders::Actions::StripCommentsAction.call(source) + + expect(source.chomp).to eq(<<~SOURCE.chomp) + method do + config.n = true + config.t = true + end + SOURCE + end + + it "removes a block of comments + newlines + other comments" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + Rails.application.configure do + config.cache_classes = true + + # Comment 1 + + # Comment 2 + config.load_defaults 6.0 + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + Rails.application.configure do + config.cache_classes = true + + config.load_defaults 6.0 + end + SOURCE + end + + it "removes a block of comments + newlines + other comments, and joins assignments" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + Rails.application.configure do + config.cache_classes = true + + # Comment 1 + + # Comment 2 + config.other = false + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + Rails.application.configure do + config.cache_classes = true + config.other = false + end + SOURCE + end + + it "removes a block of comments + newlines after indentation changes" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + Rails.application.configure do + # Comment 1 + + # Comment 2 + config.cache_classes = true + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + Rails.application.configure do + config.cache_classes = true + end + SOURCE + end + + it "removes bigger blocks of comments + newlines + other comments" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + module Foobar + class Application < Rails::Application + # Comment here + config.load_defaults 6.0 + + # Coment here + # Comment here + + # Comment here + config.generators.system_tests = nil + end + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + module Foobar + class Application < Rails::Application + config.load_defaults 6.0 + + config.generators.system_tests = nil + end + end + SOURCE + end + + it "removes leading newlines from classes, modules, and blocks" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + module Mod\n\n + class MyClass\n\n + MyGem.application.configure do |config|\n\n + config.option1 = true + + config.option2 = false + end + end + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + module Mod + class MyClass + MyGem.application.configure do |config| + config.option1 = true + config.option2 = false + end + end + end + SOURCE + end + + it "removes comments and newlines from the beginning of the source" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + # The test environment is used exclusively to run your application's + # test suite. + + Rails.application.configure do + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + Rails.application.configure do + end + SOURCE + end + + it "correctly handles removal of comments at indentation 0" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + # First require + require_relative 'boot' + + require "rails" + # Pick the frameworks you want: + require "active_model/railtie" + require "active_job/railtie" + # require "rails/test_unit/railtie" + + # Require the gems listed in Gemfile, including any gems + # you've limited to :test, :development, or :production. + Bundler.require(*Rails.groups) + + module Foo + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + require_relative 'boot' + + require "rails" + require "active_model/railtie" + require "active_job/railtie" + + Bundler.require(*Rails.groups) + + module Foo + end + SOURCE + end + + it "does not join assignments that are not preceded by other assignments" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + config.after_initialize do + Bullet.rails_logger = true + end + + config.action_mailer.delivery_method = :file + + config.action_mailer.perform_caching = false + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + config.after_initialize do + Bullet.rails_logger = true + end + + config.action_mailer.delivery_method = :file + config.action_mailer.perform_caching = false + SOURCE + end + + it "handles nested begin correctly" do + # There's an implicit "begin" block encompassing the entire code + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + # Comment here + config.action_mailer.delivery_method = :file + + # Some comment + config.action_mailer.perform_caching = false + + begin + # Comment here + config.action_mailer.first_call = :file + + # Some comment + config.action_mailer.second_call = false + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + config.action_mailer.delivery_method = :file + config.action_mailer.perform_caching = false + + begin + config.action_mailer.first_call = :file + config.action_mailer.second_call = false + end + SOURCE + end + + it "handles nested blocks correctly" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + first_block do + config.action_mailer.delivery_method = :file + + # Some comment + config.action_mailer.perform_caching = false + + second_block do + # Comment here + config.action_mailer.delivery_method = :file + + # Some comment + config.action_mailer.perform_caching = false + end + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + first_block do + config.action_mailer.delivery_method = :file + config.action_mailer.perform_caching = false + + second_block do + config.action_mailer.delivery_method = :file + config.action_mailer.perform_caching = false + end + end + SOURCE + end + + it "removes blank lines before begin" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + begin + + foo.bar = 1 + + begin + + bar.bat = 2 + end + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + begin + foo.bar = 1 + + begin + bar.bat = 2 + end + end + SOURCE + end + + it "removes comments right before the 'end' of a begin" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + begin + foo.bar = 1 + + # A comment + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + begin + foo.bar = 1 + end + SOURCE + end + + it "removes comments right before the 'end' of a block" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + foo do |instance| + instance.bar = 1 + + # A comment + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + foo do |instance| + instance.bar = 1 + end + SOURCE + end + + it "removes comments right before the 'end' of a class" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + class Foo + class Bar + # Another comment + end + + # A comment + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + class Foo + class Bar + end + end + SOURCE + end + + it "removes comments right before the 'end' of a module" do + source = Suspenders::Actions::StripCommentsAction.call(<<~SOURCE) + module Foo + module Bar + # Another comment + end + + # A comment + end + SOURCE + + expect(source.chomp).to eq(<<~SOURCE.chomp) + module Foo + module Bar + end + end + SOURCE + end + end +end diff --git a/spec/suspenders/generators/production/email_generator_spec.rb b/spec/suspenders/generators/production/email_generator_spec.rb index e35c46d03..371f7a2ed 100644 --- a/spec/suspenders/generators/production/email_generator_spec.rb +++ b/spec/suspenders/generators/production/email_generator_spec.rb @@ -10,14 +10,13 @@ end end - it "adds smtp configuration to config/smtp.rb" do + it "adds smtp configuration to production.rb" do with_fake_app do invoke! Suspenders::Production::EmailGenerator - expect("config/environments/production.rb") - .to match_contents(%r{require.+config/smtp}) - .and match_contents(%r{action_mailer.delivery_method\s*=\s*:smtp}) - .and match_contents(%r{action_mailer.smtp_settings\s*=\s*SMTP_SETTINGS}) + expect(read_file("config/environments/production.rb").lines) + .to include(" config.action_mailer.delivery_method = :smtp\n") + .and include(" config.action_mailer.smtp_settings = SMTP_SETTINGS\n") end end diff --git a/spec/suspenders/generators/staging/pull_requests_generator_spec.rb b/spec/suspenders/generators/staging/pull_requests_generator_spec.rb index d6764657a..25921389b 100644 --- a/spec/suspenders/generators/staging/pull_requests_generator_spec.rb +++ b/spec/suspenders/generators/staging/pull_requests_generator_spec.rb @@ -12,8 +12,15 @@ def setup_rails_stub(app_class_name: nil) invoke! Suspenders::Staging::PullRequestsGenerator - expect("config/environments/production.rb") - .to match_contents(%r{HEROKU_APP_NAME}) + expect(read_file("config/environments/production.rb")).to include( + [ + "", + ' if ENV.fetch("HEROKU_APP_NAME", "").include?("staging-pr-")', + ' ENV["APPLICATION_HOST"] = ENV["HEROKU_APP_NAME"] + ".herokuapp.com"', + ' ENV["ASSET_HOST"] = ENV["HEROKU_APP_NAME"] + ".herokuapp.com"', + " end" + ].join("\n") + ) end end diff --git a/suspenders.gemspec b/suspenders.gemspec index ebc0649bd..86098d016 100644 --- a/suspenders.gemspec +++ b/suspenders.gemspec @@ -27,6 +27,7 @@ Gem::Specification.new do |s| s.version = Suspenders::VERSION s.add_dependency "bitters", ">= 2.0.4" + s.add_dependency "parser", ">= 3.0" s.add_dependency "rails", Suspenders::RAILS_VERSION s.add_development_dependency "pry" diff --git a/templates/partials/email_smtp.rb b/templates/partials/email_smtp.rb index bb9944f84..eae462ba7 100644 --- a/templates/partials/email_smtp.rb +++ b/templates/partials/email_smtp.rb @@ -1,2 +1,2 @@ -config.action_mailer.delivery_method = :smtp -config.action_mailer.smtp_settings = SMTP_SETTINGS + config.action_mailer.delivery_method = :smtp + config.action_mailer.smtp_settings = SMTP_SETTINGS diff --git a/templates/partials/pull_requests_config.rb b/templates/partials/pull_requests_config.rb index a5b39f4ea..0ae37b5ab 100644 --- a/templates/partials/pull_requests_config.rb +++ b/templates/partials/pull_requests_config.rb @@ -1,4 +1,5 @@ -if ENV.fetch("HEROKU_APP_NAME", "").include?("staging-pr-") - ENV["APPLICATION_HOST"] = ENV["HEROKU_APP_NAME"] + ".herokuapp.com" - ENV["ASSET_HOST"] = ENV["HEROKU_APP_NAME"] + ".herokuapp.com" -end + if ENV.fetch("HEROKU_APP_NAME", "").include?("staging-pr-") + ENV["APPLICATION_HOST"] = ENV["HEROKU_APP_NAME"] + ".herokuapp.com" + ENV["ASSET_HOST"] = ENV["HEROKU_APP_NAME"] + ".herokuapp.com" + end +