[Fix #5823] Add new slashes style to Rails/FilePath

sunny · Jun 4, 2018 · 92fe015 · 92fe015
1 parent 7a78620
commit 92fe015
Show file tree

Hide file tree

Showing 5 changed files with 273 additions and 95 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@
 * [#5444](https://github.com/bbatsov/rubocop/pull/5444): Add new `Style/AccessModifierDeclarations` cop. ([@brandonweiss][])
 * [#5803](https://github.com/bbatsov/rubocop/issues/5803): Add new `Style/UnneededCondition` cop. ([@balbesina][])
 * [#5406](https://github.com/bbatsov/rubocop/issues/5406): Add new `Layout/ClosingHeredocIndentation` cop. ([@siggymcfried][])
+* [#5823](https://github.com/bbatsov/rubocop/issues/5823): Add new `slashes` style to `Rails/FilePath` since Ruby accepts forward slashes even on Windows. ([@sunny][])
 
 ### Bug fixes
 
@@ -3395,3 +3396,4 @@
 [@TikiTDO]: https://github.com/TikiTDO
 [@EiNSTeiN-]: https://github.com/EiNSTeiN-
 [@nroman-stripe]: https://github.com/nroman-stripe
+[@sunny]: https://github.com/sunny
diff --git a/config/default.yml b/config/default.yml
@@ -1719,6 +1719,12 @@ Rails/Exit:
   Exclude:
     - lib/**/*.rake
 
+Rails/FilePath:
+  EnforcedStyle: arguments
+  SupportedStyles:
+    - slashes
+    - arguments
+
 Rails/FindBy:
   Include:
     - app/models/**/*.rb

diff --git a/lib/rubocop/cop/rails/file_path.rb b/lib/rubocop/cop/rails/file_path.rb
@@ -4,21 +4,35 @@ module RuboCop
   module Cop
     module Rails
       # This cop is used to identify usages of file path joining process
-      # to use `Rails.root.join` clause. This is to avoid bugs on operating
-      # system that don't use '/' as the path separator.
+      # to use `Rails.root.join` clause. It is used to add uniformity when
+      # joining paths.
       #
-      # @example
-      #  # bad
-      #  Rails.root.join('app/models/goober')
-      #  File.join(Rails.root, 'app/models/goober')
-      #  "#{Rails.root}/app/models/goober"
+      # @example EnforcedStyle: arguments (default)
+      #   # bad
+      #   Rails.root.join('app/models/goober')
+      #   File.join(Rails.root, 'app/models/goober')
+      #   "#{Rails.root}/app/models/goober"
+      #
+      #   # good
+      #   Rails.root.join('app', 'models', 'goober')
+      #
+      # @example EnforcedStyle: slashes
+      #   # bad
+      #   Rails.root.join('app', 'models', 'goober')
+      #   File.join(Rails.root, 'app/models/goober')
+      #   "#{Rails.root}/app/models/goober"
+      #
+      #   # good
+      #   Rails.root.join('app/models/goober')
       #
-      #  # good
-      #  Rails.root.join('app', 'models', 'goober')
       class FilePath < Cop
+        include ConfigurableEnforcedStyle
         include RangeHelp
 
-        MSG = 'Please use `Rails.root.join(\'path\', \'to\')` instead.'.freeze
+        MSG_SLASHES = 'Please use `Rails.root.join(\'path/to\')` ' \
+                      'instead.'.freeze
+        MSG_ARGUMENTS = 'Please use `Rails.root.join(\'path\', \'to\')` ' \
+                        'instead.'.freeze
 
         def_node_matcher :file_join_nodes?, <<-PATTERN
           (send (const nil? :File) :join ...)
@@ -43,6 +57,7 @@ def on_dstr(node)
         def on_send(node)
           check_for_file_join_with_rails_root(node)
           check_for_rails_root_join_with_slash_separated_path(node)
+          check_for_rails_root_join_with_string_arguments(node)
         end
 
         private
@@ -54,7 +69,18 @@ def check_for_file_join_with_rails_root(node)
           register_offense(node)
         end
 
+        def check_for_rails_root_join_with_string_arguments(node)
+          return unless style == :slashes
+          return unless rails_root_nodes?(node)
+          return unless rails_root_join_nodes?(node)
+          return unless node.arguments.size > 1
+          return unless node.arguments.all?(&:str_type?)
+
+          register_offense(node)
+        end
+
         def check_for_rails_root_join_with_slash_separated_path(node)
+          return unless style == :arguments
           return unless rails_root_nodes?(node)
           return unless rails_root_join_nodes?(node)
           return unless node.arguments.any? { |arg| string_with_slash?(arg) }
@@ -72,6 +98,10 @@ def register_offense(node)
                                       line_range)
           add_offense(node, location: source_range)
         end
+
+        def message(_node)
+          format(style == :arguments ? MSG_ARGUMENTS : MSG_SLASHES)
+        end
       end
     end
   end

diff --git a/manual/cops_rails.md b/manual/cops_rails.md
@@ -638,11 +638,13 @@ Enabled by default | Supports autocorrection
 Enabled | No
 
 This cop is used to identify usages of file path joining process
-to use `Rails.root.join` clause. This is to avoid bugs on operating
-system that don't use '/' as the path separator.
+to use `Rails.root.join` clause. It is used to add uniformity when
+joining paths.
 
 ### Examples
 
+#### EnforcedStyle: arguments (default)
+
 ```ruby
 # bad
 Rails.root.join('app/models/goober')
@@ -652,6 +654,23 @@ File.join(Rails.root, 'app/models/goober')
 # good
 Rails.root.join('app', 'models', 'goober')
 ```
+#### EnforcedStyle: slashes
+
+```ruby
+# bad
+Rails.root.join('app', 'models', 'goober')
+File.join(Rails.root, 'app/models/goober')
+"#{Rails.root}/app/models/goober"
+
+# good
+Rails.root.join('app/models/goober')
+```
+
+### Configurable attributes
+
+Name | Default value | Configurable values
+--- | --- | ---
+EnforcedStyle | `arguments` | `slashes`, `arguments`
 
 ## Rails/FindBy