From 337fbd7c0e3ebe2fdb9b61aa28a7b7b03938cd73 Mon Sep 17 00:00:00 2001 From: "Thiago A. Silva" Date: Fri, 29 Oct 2021 16:35:37 -0300 Subject: [PATCH] Improve reliability and aesthetics of comments stripper Reliably remove comments from Rails config files when generating Suspenders apps. With the parser gem, we're now removing comments by processing their surrounding code using AST (abstract syntax tree) processors. We were using a regex to do that, which is text-based and thereby subject to edge cases that only a proper AST solution can handle. [This pull request](https://github.com/thoughtbot/suspenders/pull/1044) made changes to some config files to fix the Action Mailer asset host. While working on merging that PR, the tests were not passing because they looked for an output that was missing in one of the changed config files: ```ruby config.action_mailer.asset_host = %q{"https://#{ENV\.fetch\("ASSET_HOST", ENV\.fetch\("APPLICATION_HOST"\)\)}} ``` The PR code changes are kind of straightforward and that config option should have been present in the config file but it was not. The reason was that Suspenders was removing code comments with an overly permissive regex : (`/^.*#.*$/`), which removed every line containing a `#` character, no matter where they were in the line - so it was also removing the new Action Mailer config setting. That regex was also silently messing up with other config lines, for example: ```ruby config.public_file_server.headers = { 'Cache-Control' => "public, max-age=#{2.days.to_i}" } ``` As a quick fix to merge that PR, I've changed the regex to `/^\s*#.*$/` but it was not an ideal solution - which is what I'm striving for with this PR. With the old text-based regex solution, there were still nasty edge cases to deal with, and we may never know when they'd occur. Also, the previous regex did not work with inline comments. The final aesthetics of the config files could be improved in my opinion. This was how they looked like: ```ruby Rails.application.configure do config.cache_classes = false config.eager_load = false config.consider_all_requests_local = true if Rails.root.join('tmp', 'caching-dev.txt').exist? config.action_controller.perform_caching = true config.action_controller.enable_fragment_cache_logging = true config.cache_store = :memory_store config.public_file_server.headers = { 'Cache-Control' => "public, max-age=#{2.days.to_i}" } else config.action_controller.perform_caching = false config.cache_store = :null_store end config.active_storage.service = :local config.action_mailer.raise_delivery_errors = true config.after_initialize do Bullet.enable = true Bullet.bullet_logger = true Bullet.rails_logger = true end # ... end ``` And with the newest solution they look like this: ```ruby Rails.application.configure do config.cache_classes = false config.eager_load = false config.consider_all_requests_local = true if Rails.root.join('tmp', 'caching-dev.txt').exist? config.action_controller.perform_caching = true config.action_controller.enable_fragment_cache_logging = true config.cache_store = :memory_store config.public_file_server.headers = { 'Cache-Control' => "public, max-age=#{2.days.to_i}" } else config.action_controller.perform_caching = false config.cache_store = :null_store end config.action_mailer.raise_delivery_errors = true config.after_initialize do Bullet.enable = true Bullet.bullet_logger = true Bullet.rails_logger = true end # ... ``` Which preserves newlines where appropriate, so that's easier to read. This pull request uses the `parser` gem, and I feel like introducing that gem in the project may be a good move over the long term. For example, we need to finish up the [gem action](https://github.com/thoughtbot/suspenders/pull/1066) to sort `gem` declarations alphabetically when adding gems to the `Gemfile`, and the most reliable way to do that is with AST parsing. 1. Make the indentation of some templates match the Rails environment's config files'. This goes along with the aesthetics introduced in this PR. 2. Because of (1), I've also found a bug that was concatenaning two lines of codes in `config/environments/production.rb` with no blank line between them (in email_generator.rb), so I'm taking the opportunity to fix it 3. standardrb fixes after updating to the latest standardrb version --- .ruby-version | 2 +- .standard.yml | 2 + lib/suspenders.rb | 1 + lib/suspenders/actions.rb | 2 +- .../actions/strip_comments_action.rb | 254 +++++++++ lib/suspenders/app_builder.rb | 11 +- .../generators/production/email_generator.rb | 4 +- spec/support/file_operations.rb | 4 + .../actions/strip_comments_action_spec.rb | 489 ++++++++++++++++++ .../production/email_generator_spec.rb | 9 +- .../staging/pull_requests_generator_spec.rb | 11 +- suspenders.gemspec | 1 + templates/partials/email_smtp.rb | 4 +- templates/partials/pull_requests_config.rb | 9 +- 14 files changed, 778 insertions(+), 25 deletions(-) create mode 100644 lib/suspenders/actions/strip_comments_action.rb create mode 100644 spec/suspenders/actions/strip_comments_action_spec.rb 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 +