Suggested changes

Gemfile

@@ -3,7 +3,7 @@ source "https://rubygems.org"
 ruby "2.6.5"
 
 gem "active_model_serializers", "0.10.10"
-gem "administrate", "0.12.0"
+gem "administrate", "0.13.0"
 gem "analytics-ruby", "~> 2.2.2", require: "segment/analytics"
 gem "attr_extras"
 gem "autoprefixer-rails"

Gemfile.lock

@@ -1,38 +1,38 @@
 GEM
   remote: https://rubygems.org/
   specs:
-    actioncable (6.0.2.1)
-      actionpack (= 6.0.2.1)
+    actioncable (6.0.2.2)
+      actionpack (= 6.0.2.2)
       nio4r (~> 2.0)
       websocket-driver (>= 0.6.1)
-    actionmailbox (6.0.2.1)
-      actionpack (= 6.0.2.1)
-      activejob (= 6.0.2.1)
-      activerecord (= 6.0.2.1)
-      activestorage (= 6.0.2.1)
-      activesupport (= 6.0.2.1)
+    actionmailbox (6.0.2.2)
+      actionpack (= 6.0.2.2)
+      activejob (= 6.0.2.2)
+      activerecord (= 6.0.2.2)
+      activestorage (= 6.0.2.2)
+      activesupport (= 6.0.2.2)
       mail (>= 2.7.1)
-    actionmailer (6.0.2.1)
-      actionpack (= 6.0.2.1)
-      actionview (= 6.0.2.1)
-      activejob (= 6.0.2.1)
+    actionmailer (6.0.2.2)
+      actionpack (= 6.0.2.2)
+      actionview (= 6.0.2.2)
+      activejob (= 6.0.2.2)
       mail (~> 2.5, >= 2.5.4)
       rails-dom-testing (~> 2.0)
-    actionpack (6.0.2.1)
-      actionview (= 6.0.2.1)
-      activesupport (= 6.0.2.1)
+    actionpack (6.0.2.2)
+      actionview (= 6.0.2.2)
+      activesupport (= 6.0.2.2)
       rack (~> 2.0, >= 2.0.8)
       rack-test (>= 0.6.3)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.0, >= 1.2.0)
-    actiontext (6.0.2.1)
-      actionpack (= 6.0.2.1)
-      activerecord (= 6.0.2.1)
-      activestorage (= 6.0.2.1)
-      activesupport (= 6.0.2.1)
+    actiontext (6.0.2.2)
+      actionpack (= 6.0.2.2)
+      activerecord (= 6.0.2.2)
+      activestorage (= 6.0.2.2)
+      activesupport (= 6.0.2.2)
       nokogiri (>= 1.8.5)
-    actionview (6.0.2.1)
-      activesupport (= 6.0.2.1)
+    actionview (6.0.2.2)
+      activesupport (= 6.0.2.2)
       builder (~> 3.1)
       erubi (~> 1.4)
       rails-dom-testing (~> 2.0)
@@ -42,28 +42,28 @@ GEM
       activemodel (>= 4.1, < 6.1)
       case_transform (>= 0.2)
       jsonapi-renderer (>= 0.1.1.beta1, < 0.3)
-    activejob (6.0.2.1)
-      activesupport (= 6.0.2.1)
+    activejob (6.0.2.2)
+      activesupport (= 6.0.2.2)
       globalid (>= 0.3.6)
-    activemodel (6.0.2.1)
-      activesupport (= 6.0.2.1)
-    activerecord (6.0.2.1)
-      activemodel (= 6.0.2.1)
-      activesupport (= 6.0.2.1)
-    activestorage (6.0.2.1)
-      actionpack (= 6.0.2.1)
-      activejob (= 6.0.2.1)
-      activerecord (= 6.0.2.1)
+    activemodel (6.0.2.2)
+      activesupport (= 6.0.2.2)
+    activerecord (6.0.2.2)
+      activemodel (= 6.0.2.2)
+      activesupport (= 6.0.2.2)
+    activestorage (6.0.2.2)
+      actionpack (= 6.0.2.2)
+      activejob (= 6.0.2.2)
+      activerecord (= 6.0.2.2)
       marcel (~> 0.3.1)
-    activesupport (6.0.2.1)
+    activesupport (6.0.2.2)
       concurrent-ruby (~> 1.0, >= 1.0.2)
       i18n (>= 0.7, < 2)
       minitest (~> 5.1)
       tzinfo (~> 1.1)
       zeitwerk (~> 2.2)
     addressable (2.7.0)
       public_suffix (>= 2.0.2, < 5.0)
-    administrate (0.12.0)
+    administrate (0.13.0)
       actionpack (>= 4.2)
       actionview (>= 4.2)
       activerecord (>= 4.2)
@@ -76,7 +76,7 @@ GEM
       selectize-rails (~> 0.6)
     analytics-ruby (2.2.8)
     attr_extras (6.2.3)
-    autoprefixer-rails (9.7.4)
+    autoprefixer-rails (9.7.5)
       execjs
     bourbon (5.1.0)
       sass (~> 3.4)
@@ -175,7 +175,7 @@ GEM
       mini_mime (>= 0.1.1)
     marcel (0.3.3)
       mimemagic (~> 0.3.2)
-    method_source (0.9.2)
+    method_source (1.0.0)
     mimemagic (0.3.4)
     mini_mime (1.0.2)
     mini_portile2 (2.4.0)
@@ -191,7 +191,7 @@ GEM
       sass (>= 3.3)
       thor (~> 0.19)
     nio4r (2.5.2)
-    nokogiri (1.10.8)
+    nokogiri (1.10.9)
       mini_portile2 (~> 2.4.0)
     normalize-rails (4.1.1)
     oauth2 (1.4.4)
@@ -220,7 +220,7 @@ GEM
     pathspec (0.2.1)
     pg (1.2.2)
     public_suffix (4.0.3)
-    puma (4.3.1)
+    puma (4.3.3)
       nio4r (~> 2.0)
     rack (2.2.2)
     rack-protection (2.0.8.1)
@@ -230,20 +230,20 @@ GEM
     rack-test (1.1.0)
       rack (>= 1.0, < 3)
     rack-timeout (0.6.0)
-    rails (6.0.2.1)
-      actioncable (= 6.0.2.1)
-      actionmailbox (= 6.0.2.1)
-      actionmailer (= 6.0.2.1)
-      actionpack (= 6.0.2.1)
-      actiontext (= 6.0.2.1)
-      actionview (= 6.0.2.1)
-      activejob (= 6.0.2.1)
-      activemodel (= 6.0.2.1)
-      activerecord (= 6.0.2.1)
-      activestorage (= 6.0.2.1)
-      activesupport (= 6.0.2.1)
+    rails (6.0.2.2)
+      actioncable (= 6.0.2.2)
+      actionmailbox (= 6.0.2.2)
+      actionmailer (= 6.0.2.2)
+      actionpack (= 6.0.2.2)
+      actiontext (= 6.0.2.2)
+      actionview (= 6.0.2.2)
+      activejob (= 6.0.2.2)
+      activemodel (= 6.0.2.2)
+      activerecord (= 6.0.2.2)
+      activestorage (= 6.0.2.2)
+      activesupport (= 6.0.2.2)
       bundler (>= 1.3.0)
-      railties (= 6.0.2.1)
+      railties (= 6.0.2.2)
       sprockets-rails (>= 2.0.0)
     rails-dom-testing (2.0.3)
       activesupport (>= 4.2.0)
@@ -255,9 +255,9 @@ GEM
       rails_stdout_logging
     rails_serve_static_assets (0.0.5)
     rails_stdout_logging (0.0.5)
-    railties (6.0.2.1)
-      actionpack (= 6.0.2.1)
-      activesupport (= 6.0.2.1)
+    railties (6.0.2.2)
+      actionpack (= 6.0.2.2)
+      activesupport (= 6.0.2.2)
       method_source
       rake (>= 0.8.7)
       thor (>= 0.20.3, < 2.0)
@@ -364,14 +364,14 @@ GEM
     websocket-extensions (0.1.4)
     xpath (3.2.0)
       nokogiri (~> 1.8)
-    zeitwerk (2.2.2)
+    zeitwerk (2.3.0)
 
 PLATFORMS
   ruby
 
 DEPENDENCIES
   active_model_serializers (= 0.10.10)
-  administrate (= 0.12.0)
+  administrate (= 0.13.0)
   analytics-ruby (~> 2.2.2)
   attr_extras
   autoprefixer-rails

app/models/file_review.rb

@@ -2,11 +2,12 @@ class FileReview < ApplicationRecord
   belongs_to :build
   has_many :violations, dependent: :destroy
 
-  def build_violation(line, message)
+  def build_violation(line, message, source)
     if line.changed?
       violation = find_or_build_violation(line)
       violation.add_message(message)
       violation.patch_position = line.patch_position
+      violation.source = source
     end
   end
 

app/models/user.rb

@@ -38,15 +38,11 @@ def has_active_repos?
   end
 
   def token=(value)
-    encrypted_token = crypt.encrypt_and_sign(value)
-    write_attribute(:token, encrypted_token)
+    self[:token] = crypt.encrypt_and_sign(value)
   end
 
   def token
-    encrypted_token = read_attribute(:token)
-    unless encrypted_token.nil?
-      crypt.decrypt_and_verify(encrypted_token)
-    end
+    crypt.decrypt_and_verify(self[:token]) if self[:token]
   end
 
   def payment_gateway_subscription

app/models/violation.rb

@@ -14,4 +14,19 @@ def add_message(message)
   def messages
     self[:messages].uniq
   end
+
+  def source=(value)
+    self[:source] = crypt.encrypt_and_sign(value)
+  end
+
+  def source
+    crypt.decrypt_and_verify(self[:source]) if self[:source]
+  end
+
+  private
+
+  def crypt
+    secret_key_base = Rails.application.secrets.secret_key_base
+    ActiveSupport::MessageEncryptor.new(secret_key_base[0, 32], secret_key_base)
+  end
 end

app/services/complete_file_review.rb

@@ -27,7 +27,12 @@ def complete_file_review
   def build_file_review_violations
     attributes.fetch(:violations).each do |violation|
       line = commit_file.line_at(violation.fetch(:line))
-      file_review.build_violation(line, violation.fetch(:message))
+
+      file_review.build_violation(
+        line,
+        violation.fetch(:message),
+        violation.fetch(:source),
+      )
     end
   end
 

app/services/submit_review.rb

@@ -43,7 +43,7 @@ def build_comment(violation)
     {
       path: violation.filename,
       position: violation.patch_position,
-      body: violation.messages.join(CommentingPolicy::COMMENT_LINE_DELIMITER),
+      body: SuggestChanges.call(violation),
     }
   end
 

app/services/suggest_changes.rb

@@ -0,0 +1,41 @@
+class SuggestChanges
+  static_facade :call
+
+  def initialize(violation)
+    @violation = violation
+    @suggestion = ""
+  end
+
+  def call
+    violation.messages.each do |message|
+      apply_suggestion(message)
+      break if suggestion.present?
+    end
+
+    messages = violation.messages.join(CommentingPolicy::COMMENT_LINE_DELIMITER)
+
+    if suggestion.blank?
+      messages
+    else
+      messages + "<br>\n```suggestion\n#{suggestion}\n```"
+    end
+  end
+
+  private
+
+  attr_reader :violation
+  attr_accessor :suggestion
+
+  def apply_suggestion(message)
+    case message
+    when /Trailing whitespace detected/
+      self.suggestion = violation.source.gsub(/(.*)\s$/, '\1')
+    when /A space is required after ','/
+      self.suggestion = violation.source.gsub(/(,)([^ ])/, ', \2')
+    when /Put a comma after the last parameter of a multiline method call/
+      self.suggestion = violation.source + ","
+    when /Missing semicolon/
+      self.suggestion = violation.source + ";"
+    end
+  end
+end

db/migrate/20191108222026_add_source_to_violations.rb

@@ -0,0 +1,5 @@
+class AddSourceToViolations < ActiveRecord::Migration[5.1]
+  def change
+    add_column :violations, :source, :string
+  end
+end

db/schema.rb

@@ -138,6 +138,7 @@
     t.integer "line_number"
     t.text "messages", default: [], null: false, array: true
     t.integer "file_review_id", null: false
+    t.string "source"
     t.index ["file_review_id"], name: "index_violations_on_file_review_id"
   end
 

doc/SECURITY.md

@@ -218,9 +218,11 @@ to comment about the violations on the pull request.
 
 `CompleteFileReview` saves each violation, the line number, and patch position,
 in the `violations` table of our Postgres database.
-We do not save any of your code in Postgres.
-We do store the contents of the files to review temporarily in Sidekiq, and it
-gets cleared out as soon as the job is processed.
+We also store single lines of code that are used to report violations and
+suggested changes. The data is encrypted and encoded similar to tokens using
+`ActiveSupport::MessageEncryptor`. We also store the contents of the files to
+review temporarily in Redis, but that is removed as soon as the Sidekiq job is
+processed.
 
 To browse the portions of the codebase related to
 receiving and processing pull request notifications,