Makes file extensions configurable (#8)

* Allow user to specify file extensions to check * Improve code style violation message and refactor its generation * Adds code style violation message verification * Adds .clang-format to make diffs produced in tests deterministic * Updates specs to fix #9 * Install clang-format-3.8 also on Linux
flix-tech · Aug 18, 2017 · 5c250d9 · 5c250d9
1 parent a9dc5f4
commit 5c250d9
Show file tree

Hide file tree

Showing 6 changed files with 114 additions and 35 deletions.
diff --git a/.clang-format b/.clang-format
@@ -0,0 +1 @@
+BasedOnStyle: LLVM
diff --git a/.travis.yml b/.travis.yml
@@ -19,7 +19,7 @@ branches:
 addons:
   apt:
     packages: [
-      clang-format-3.6
+      clang-format-3.8
     ]
 
 before_install:

diff --git a/README.md b/README.md
@@ -2,9 +2,10 @@
 
 [![Build Status](https://travis-ci.org/flix-tech/danger-code_style_validation.svg?branch=master)](https://travis-ci.org/flix-tech/danger-code_style_validation)
 
-This plugin looks for code style violations for added lines and suggests patches.
-
-It uses 'clang-format' and only checks `.h`, `.m` and `.mm` files
+This plugin uses 'clang-format' to look for code style violations in added
+lines on the current MR / PR, and offers inline patches.
+By default only Objective-C files, with extensions `.h`, `.m`, and `.mm` are
+checked.
 
 ![Example](/doc/images/example.png)
 
@@ -24,6 +25,12 @@ Inside your `Dangerfile` :
 code_style_validation.check
 ```
 
+To check files with extensions other than the default ones:
+
+```ruby
+code_style_validation.check file_extensions: ['.hpp', '.cpp']
+```
+
 To ignore specific paths, use `ignore_file_patterns` :
 
 ```ruby

diff --git a/lib/code_style_validation/plugin.rb b/lib/code_style_validation/plugin.rb
@@ -1,15 +1,18 @@
 module Danger
-  # This plugin looks for code style violations for
-  # added lines on the current MR / PR,
-  # and offers inline patches.
+  # This plugin uses 'clang-format' to look for code style violations in added
+  # lines on the current MR / PR, and offers inline patches.
+  # By default only Objective-C files, with extensions ".h", ".m", and ".mm"
+  # are checked.
   #
-  # It uses 'clang-format' and only checks ".h", ".m" and ".mm" files
-  #
-  # @example Ensure that added lines does not violate code style
+  # @example Ensure that changes do not violate code style in Objective-C files
   #
   #          code_style_validation.check
   #
-  # @example Ensure that changes don't violate code style, ignoring Pods directory
+  # @example Ensure that changes do not violate code style in files with given extensions
+  #
+  #          code_style_validation.check file_extensions: ['.hpp', '.cpp']
+  #
+  # @example Ensure that changes do not violate code style, ignoring Pods directory
   #
   #          code_style_validation.check ignore_file_patterns: [/^Pods\//]
   #
@@ -18,12 +21,15 @@ module Danger
   #
   class DangerCodeStyleValidation < Plugin
     VIOLATION_ERROR_MESSAGE = 'Code style violations detected.'.freeze
-    
+
     # Validates the code style of changed & added files using clang-format.
     # Generates Markdown message with respective patches.
     #
     # @return [void]
     def check(config = {})
+      defaults = {file_extensions: ['.h', '.m', '.mm'], ignore_file_patterns: []}
+      config = defaults.merge(config)
+      file_extensions = [*config[:file_extensions]]
       ignore_file_patterns = [*config[:ignore_file_patterns]]
 
       diff = ''
@@ -38,19 +44,31 @@ def check(config = {})
         raise 'Unknown SCM Provider'
       end
 
-      changes = get_changes(diff, ignore_file_patterns)
-      message = resolve_changes(changes)
+      changes = get_changes(diff, file_extensions, ignore_file_patterns)
+      offending_files, patches = resolve_changes(changes)
+
+      message = ''
+      unless offending_files.empty?
+        message = 'Code style violations detected in the following files:' + "\n"
+        offending_files.each do |file_name|
+          message += '* `' + file_name + "`\n\n"
+        end
+        message += 'Execute one of the following actions and commit again:' + "\n"
+        message += '1. Run `clang-format` on the offending files' + "\n"
+        message += '2. Apply the suggested patches with `git apply patch`.' + "\n\n"
+        message += patches.join("\n")
+      end
 
       return if message.empty?
       fail VIOLATION_ERROR_MESSAGE
-      markdown '### Code Style Check (`.h`, `.m` and `.mm`)'
+      markdown '### Code Style Check'
       markdown '---'
       markdown message
     end
 
     private
 
-    def get_changes(diff_str, ignore_file_patterns)
+    def get_changes(diff_str, file_extensions, ignore_file_patterns)
       changes = {}
       line_cursor = 0
 
@@ -69,7 +87,7 @@ def get_changes(diff_str, ignore_file_patterns)
 
         file_name = filename_line.split('+++ b/').last.chomp
 
-        unless file_name.end_with?('.m', '.h', '.mm')
+        unless file_name.end_with?(*file_extensions)
           next
         end
 
@@ -121,17 +139,17 @@ def parse_diff(diff)
       patches
     end
 
-    def generate_markdown(title, content)
-      markup_message = '#### ' + title + "\n"
-      markup_message += "```diff \n" + content + "\n``` \n"
-      markup_message
+    def generate_patch(title, content)
+      markup_patch = '#### ' + title + "\n"
+      markup_patch += "```diff \n" + content + "\n``` \n"
+      markup_patch
     end
 
     def resolve_changes(changes)
       # Parse all patches from diff string
 
-      markup_message = ''
-
+      offending_files = []
+      patches = []
       # patches.each do |patch|
       changes.each do |file_name, changed_lines|
         changed_lines_command_array = []
@@ -157,14 +175,16 @@ def resolve_changes(changes)
         formatted_temp_file.close
         formatted_temp_file.unlink
 
-        # generate Markup message of patch suggestions
-        # to prevent code-style violations
+        # generate arrays with:
+        # 1. Name of offending files
+        # 2. Suggested patches, in Markdown format
         unless diff.empty?
-          markup_message += generate_markdown(file_name, diff)
+          offending_files.push(file_name)
+          patches.push(generate_patch(file_name, diff))
         end
       end
 
-      markup_message
+      return offending_files, patches
     end
   end
 end
diff --git a/spec/code_style_validation_spec.rb b/spec/code_style_validation_spec.rb
@@ -14,17 +14,28 @@ module Danger
 
       it 'Reports code style violation as error' do
         diff = File.read('spec/fixtures/violated_diff.diff')
+        expected_message = File.read('spec/fixtures/violated_diff_message.md')
 
-        @my_plugin.github.stub(:pr_diff).and_return diff
-        @my_plugin.check
+        allow(@my_plugin.github).to receive(:pr_diff).and_return diff
+        @my_plugin.check file_extensions: ['.h', '.m', '.mm', '.cpp']
 
         expect(@dangerfile.status_report[:errors]).to eq([DangerCodeStyleValidation::VIOLATION_ERROR_MESSAGE])
+        expect(@dangerfile.status_report[:markdowns].map(&:message).join("\n")).to eq(expected_message)
+      end
+
+      it 'Does not report error when extension is excluded' do
+        diff = File.read('spec/fixtures/violated_diff.diff')
+
+        allow(@my_plugin.github).to receive(:pr_diff).and_return diff
+        @my_plugin.check file_extensions: ['.h', '.c']
+
+        expect(@dangerfile.status_report[:errors]).to eq([])
       end
 
       it 'Does not report error when code not violated' do
         diff = File.read('spec/fixtures/innocent_diff.diff')
 
-        @my_plugin.github.stub(:pr_diff).and_return diff
+        allow(@my_plugin.github).to receive(:pr_diff).and_return diff
         @my_plugin.check
 
         expect(@dangerfile.status_report[:errors]).to eq([])
@@ -33,7 +44,7 @@ module Danger
       it 'Does not report error for different extension types of files' do
         diff = File.read('spec/fixtures/ruby_diff.diff')
 
-        @my_plugin.github.stub(:pr_diff).and_return diff
+        allow(@my_plugin.github).to receive(:pr_diff).and_return diff
         @my_plugin.check
 
         expect(@dangerfile.status_report[:errors]).to eq([])
@@ -42,7 +53,7 @@ module Danger
       it 'Does not report unexpected errors when there are only removals in the diff' do
         diff = File.read('spec/fixtures/red_diff.diff')
 
-        @my_plugin.github.stub(:pr_diff).and_return diff
+        allow(@my_plugin.github).to receive(:pr_diff).and_return diff
         @my_plugin.check
 
         expect(@dangerfile.status_report[:errors]).to eq([])
@@ -51,20 +62,30 @@ module Danger
       it 'Ignores files matching ignored patterns' do
         diff = File.read('spec/fixtures/violated_diff.diff')
 
-        @my_plugin.github.stub(:pr_diff).and_return diff
-        @my_plugin.check ignore_file_patterns: [%r{^spec/}]
+        allow(@my_plugin.github).to receive(:pr_diff).and_return diff
+        @my_plugin.check file_extensions: ['.h', '.m'],
+                         ignore_file_patterns: [%r{^spec/}]
 
         expect(@dangerfile.status_report[:errors]).to eq([])
       end
 
       it 'Allows single pattern instead of array' do
         diff = File.read('spec/fixtures/violated_diff.diff')
 
-        @my_plugin.github.stub(:pr_diff).and_return diff
+        allow(@my_plugin.github).to receive(:pr_diff).and_return diff
         @my_plugin.check ignore_file_patterns: %r{^spec/}
 
         expect(@dangerfile.status_report[:errors]).to eq([])
       end
+
+      it 'Allows single file extension instead of array' do
+        diff = File.read('spec/fixtures/violated_diff.diff')
+
+        allow(@my_plugin.github).to receive(:pr_diff).and_return diff
+        @my_plugin.check file_extensions: '.m'
+
+        expect(@dangerfile.status_report[:errors]).to eq([DangerCodeStyleValidation::VIOLATION_ERROR_MESSAGE])
+      end
     end
   end
 end
diff --git a/spec/fixtures/violated_diff_message.md b/spec/fixtures/violated_diff_message.md
@@ -0,0 +1,30 @@
+### Code Style Check
+---
+Code style violations detected in the following files:
+* `spec/fixtures/BadViewController.m`
+
+Execute one of the following actions and commit again:
+1. Run `clang-format` on the offending files
+2. Apply the suggested patches with `git apply patch`.
+
+#### spec/fixtures/BadViewController.m
+```diff 
+--- spec/fixtures/BadViewController.m
++++ spec/fixtures/BadViewController.m
+@@ -1,9 +1,10 @@
+-@interface ViewController (  ) @end
++@interface ViewController ()
++@end
+
+ @implementation ViewController
+--(void ) viewDidLoad {
+-    [super viewDidLoad];
+-    NSLog(  @"perfect change!")   ;
++- (void)viewDidLoad {
++  [super viewDidLoad];
++  NSLog(@"perfect change!");
+ }
+
+ @end
+
+```