Merge pull request #10 from poise/pr/5

Updated PR: FC013 Use file_cache_path

.travis.yml

@@ -1,7 +1,16 @@
 sudo: false
 cache: bundler
 language: ruby
-rvm: "2.3.1"
+rvm:
+  - 2.3.5
+  - 2.4.2
+  - ruby-head
+matrix:
+  allow_failures:
+    - rvm: ruby-head
+branches:
+  only:
+    - master
 before_install:
 - if [[ $BUNDLE_GEMFILE == *master.gemfile ]]; then gem update --system; fi
 - gem --version

README.md

@@ -10,7 +10,7 @@ Chef-specific analysis for your projects, as an extension to
 * ~~FC001 Use strings in preference to symbols to access node attributes~~ Chef/AttributeKeys
 * ~~FC002 Avoid string interpolation where not required~~ Style/UnneededInterpolation
 * ~~FC003 Check whether you are running with chef server before using server-specific features~~ Deprecated because solo supports search
-* FC004 Use a service resource to start and stop services
+* ~~FC004 Use a service resource to start and stop services~~
 * FC005 Avoid repetition of resource declarations
 * ~~FC006 Mode should be quoted or fully specified when setting file permissions~~ Chef/FileMode
 * FC007 Ensure recipe dependencies are reflected in cookbook metadata
@@ -19,7 +19,7 @@ Chef-specific analysis for your projects, as an extension to
 * FC010 Invalid search syntax
 * FC011 Missing README in markdown format
 * FC012 Use Markdown for README rather than RDoc
-* FC013 Use file_cache_path rather than hard-coding tmp paths
+* ~~FC013 Use file_cache_path rather than hard-coding tmp paths~~
 * FC014 Consider extracting long ruby_block to library
 * FC015 Consider converting definition to a Custom Resource
 * FC016 LWRP does not declare a default action

config/default.yml

@@ -34,6 +34,10 @@ Chef/ServiceResource:
   Description: Use a service resource to start and stop services
   Enabled: true
 
+Chef/TmpPath:
+  Description: Use file_cache_path rather than hard-coding tmp paths
+  Enabled: true
+
 Chef/CopyrightCommentFormat:
   Description: Properly format copyright dates in comment blocks and ensure dates are up to date
   Enabled: false

lib/rubocop-chef.rb

@@ -15,9 +15,10 @@
 
 RuboCop::Chef::Inject.defaults!
 
-# cops
+# Chef specific cops
 require 'rubocop/cop/chef/attribute_keys'
 require 'rubocop/cop/chef/file_mode'
 require 'rubocop/cop/chef/service_resource'
 require 'rubocop/cop/chef/comments_format'
 require 'rubocop/cop/chef/comments_copyright_format'
+require 'rubocop/cop/chef/tmp_path'

lib/rubocop/cop/chef/attribute_keys.rb

@@ -68,25 +68,24 @@ def on_send(node)
         def on_node_attribute_access(node)
           if node.type == :str
             style_detected(:strings)
-            add_offense(node, :expression, MSG % style) if style == :symbols
+            add_offense(node, location: :expression, message: MSG % style) if style == :symbols
           elsif node.type == :sym
             style_detected(:symbols)
-            add_offense(node, :expression, MSG % style) if style == :strings
+            add_offense(node, location: :expression, message: MSG % style) if style == :strings
           end
         end
 
         def autocorrect(node)
           lambda do |corrector|
             key_string = node.children.first.to_s
             key_replacement = if style == :symbols
-              key_string.to_sym.inspect
-            else # strings
-              key_string.inspect
+                                key_string.to_sym.inspect
+                              else # strings
+                                key_string.inspect
             end
             corrector.replace(node.loc.expression, key_replacement)
           end
         end
-
       end
     end
   end

lib/rubocop/cop/chef/comments_copyright_format.rb

@@ -46,7 +46,7 @@ def investigate(processed_source)
             next unless comment.inline? # headers aren't in blocks
 
             if invalid_copyright_comment?(comment)
-              add_offense(comment, comment.loc.expression, MSG)
+              add_offense(comment, location: comment.loc.expression, message: MSG)
             end
           end
         end

lib/rubocop/cop/chef/comments_format.rb

@@ -46,7 +46,7 @@ def investigate(processed_source)
             next unless comment.inline? # headers aren't in blocks
 
             if invalid_comment?(comment)
-              add_offense(comment, comment.loc.expression, MSG)
+              add_offense(comment, location: comment.loc.expression, message: MSG)
             end
           end
         end

lib/rubocop/cop/chef/file_mode.rb

@@ -37,7 +37,7 @@ class FileMode < Cop
 
         def on_send(node)
           resource_mode?(node) do |mode_int|
-            add_offense(mode_int, :expression, MSG, is_octal?(mode_int) ? :warning : :error)
+            add_offense(mode_int, location: :expression, message: MSG, severity: is_octal?(mode_int) ? :warning : :error)
           end
         end
 
@@ -55,7 +55,6 @@ def autocorrect(node)
         def is_octal?(node)
           node.source =~ /^0o?\d+/i
         end
-
       end
     end
   end

lib/rubocop/cop/chef/service_resource.rb

@@ -26,7 +26,6 @@ module Chef
       #   command "/sbin/service/memcached start"
       #
       class ServiceResource < Cop
-
         MSG = 'Use a service resource to start and stop services'.freeze
 
         def_node_matcher :execute_command?, <<-PATTERN
@@ -36,19 +35,18 @@ class ServiceResource < Cop
         def on_send(node)
           execute_command?(node) do |command|
             if starts_service?(command)
-              add_offense(command, :expression, MSG, :error)
+              add_offense(command, location: :expression, message: MSG, severity: :error)
             end
           end
         end
 
         def starts_service?(cmd)
           cmd_str = cmd.to_s
-          (cmd_str.include?("/etc/init.d") || ["service ", "/sbin/service ",
-                 "start ", "stop ", "invoke-rc.d "].any? do |service_cmd|
-                   cmd_str.start_with?(service_cmd)
-                 end) && %w{start stop restart reload}.any? { |a| cmd_str.include?(a) }
+          (cmd_str.include?('/etc/init.d') || ['service ', '/sbin/service ',
+                                               'start ', 'stop ', 'invoke-rc.d '].any? do |service_cmd|
+             cmd_str.start_with?(service_cmd)
+           end) && %w[start stop restart reload].any? { |a| cmd_str.include?(a) }
         end
-
       end
     end
   end

lib/rubocop/cop/chef/tmp_path.rb

@@ -0,0 +1,58 @@
+#
+# Copyright 2016, Chris Henry
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+module RuboCop
+  module Cop
+    module Chef
+      # Use file_cache_path rather than hard-coding tmp paths
+      #
+      # @example downloading a large file into /tmp/
+      #
+      #   # bad
+      #   remote_file '/tmp/large-file.tar.gz' do
+      #
+      #   # good
+      #   remote_file "#{Chef::Config[:file_cache_path]}/large-file.tar.gz" do
+      #
+      #
+      class TmpPath < Cop
+        MSG = 'Use file_cache_path rather than hard-coding tmp paths'.freeze
+
+        def_node_matcher :remote_file?, <<-PATTERN
+          (send nil :remote_file $str)
+        PATTERN
+
+        def on_send(node)
+          remote_file?(node) do |command|
+            if has_hardcoded_tmp?(command) && !has_file_cache_path?(command)
+              add_offense(command, location: :expression, message: MSG, severity: :error)
+            end
+          end
+        end
+
+        def has_hardcoded_tmp?(path)
+          path_str = path.to_s.scan(/"(.*)"/)[0][0]
+          path_str.start_with?('/tmp/')
+        end
+
+        def has_file_cache_path?(path)
+          path_str = path.to_s.scan(/"(.*)"/)[0][0]
+          path_str.start_with?("\#\{Chef::Config[:file_cache_path]\}")
+        end
+      end
+    end
+  end
+end

rubocop-chef.gemspec

@@ -1,5 +1,3 @@
-# encoding: utf-8
-
 $LOAD_PATH.unshift File.expand_path('../lib', __FILE__)
 require 'rubocop/chef/version'
 
@@ -29,11 +27,11 @@ Gem::Specification.new do |spec|
 
   spec.add_runtime_dependency 'rubocop', '>= 0.39'
 
+  spec.add_development_dependency 'adamantium'
+  spec.add_development_dependency 'anima'
+  spec.add_development_dependency 'concord'
   spec.add_development_dependency 'rake'
   spec.add_development_dependency 'rspec', '>= 3.4'
   spec.add_development_dependency 'simplecov'
-  spec.add_development_dependency 'anima'
-  spec.add_development_dependency 'concord'
-  spec.add_development_dependency 'adamantium'
   spec.add_development_dependency 'yard'
 end

spec/rubocop/chef/cookbook_only_spec.rb

@@ -29,7 +29,7 @@ def self.name
       end
 
       def on_send(node)
-        add_offense(node, :expression, 'boom')
+        add_offense(node, location: :expression, message: 'boom')
       end
     end
   end
@@ -43,7 +43,7 @@ def on_send(node)
     end
 
     context 'with the metadata segment enabled' do
-      let(:segments) { {metadata: true} }
+      let(:segments) { { metadata: true } }
 
       it 'registers an offense' do
         expect_violation(<<-RUBY, filename: 'metadata.rb')
@@ -54,7 +54,7 @@ def on_send(node)
     end # /context with the metadata segment enabled
 
     context 'with the metadata segment disabled' do
-      let(:segments) { {metadata: false} }
+      let(:segments) { { metadata: false } }
 
       it 'ignores the source' do
         expect_no_violations(<<-RUBY, filename: 'metadata.rb')
@@ -73,7 +73,7 @@ def on_send(node)
     end
 
     context 'with the recipes segment enabled' do
-      let(:segments) { {recipes: true} }
+      let(:segments) { { recipes: true } }
 
       it 'registers an offense' do
         expect_violation(<<-RUBY, filename: 'recipes/default.rb')
@@ -84,7 +84,7 @@ def on_send(node)
     end # /context with the recipes segment enabled
 
     context 'with the recipes segment disabled' do
-      let(:segments) { {recipes: false} }
+      let(:segments) { { recipes: false } }
 
       it 'ignores the source' do
         expect_no_violations(<<-RUBY, filename: 'recipes/default.rb')
@@ -95,14 +95,14 @@ def on_send(node)
   end # /context when the source path is recipes/default.rb
 
   context 'when the source path is recipes/ignored.rb' do
-      it 'ignores the source' do
-        expect_no_violations(<<-RUBY, filename: 'recipes/ignored.rb')
-          foo(1)
-        RUBY
-      end
+    it 'ignores the source' do
+      expect_no_violations(<<-RUBY, filename: 'recipes/ignored.rb')
+        foo(1)
+      RUBY
+    end
 
     context 'with the recipes segment enabled' do
-      let(:segments) { {recipes: true} }
+      let(:segments) { { recipes: true } }
 
       it 'ignores the source' do
         expect_no_violations(<<-RUBY, filename: 'recipes/ignored.rb')
@@ -112,7 +112,7 @@ def on_send(node)
     end # /context with the recipes segment enabled
 
     context 'with the recipes segment disabled' do
-      let(:segments) { {recipes: false} }
+      let(:segments) { { recipes: false } }
 
       it 'ignores the source' do
         expect_no_violations(<<-RUBY, filename: 'recipes/ignored.rb')
@@ -124,7 +124,7 @@ def on_send(node)
 
   context 'when custom patterns are specified' do
     let(:all_cop_config) do
-      {'ChefRecipes' => {'Patterns' => ['chef/recipes/.*\\.rb']}}
+      { 'ChefRecipes' => { 'Patterns' => ['chef/recipes/.*\\.rb'] } }
     end
 
     it 'registers offenses when the path matches a custom specified pattern' do

spec/rubocop/cop/chef/service_resource_spec.rb

@@ -35,5 +35,4 @@
       end
     RUBY
   end
-
 end

spec/rubocop/cop/chef/tmp_path_spec.rb

@@ -0,0 +1,38 @@
+#
+# Copyright 2016, Chris Henry
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'spec_helper'
+
+describe RuboCop::Cop::Chef::TmpPath, :config do
+  subject(:cop) { described_class.new(config) }
+
+  it 'registers an offense when hardcoding a path in /tmp' do
+    expect_violation(<<-RUBY)
+      remote_file '/tmp/large-file.tar.gz' do
+                  ^^^^^^^^^^^^^^^^^^^^^^^^ Use file_cache_path rather than hard-coding tmp paths
+        source 'http://www.example.org/large-file.tar.gz'
+      end
+    RUBY
+  end
+
+  it "doesn't register an offense when using file_cache_path" do
+    expect_no_violations(<<-RUBY)
+      remote_file "\#\{Chef::Config[:file_cache_path]\}/large-file.tar.gz" do
+        source 'http://www.example.org/large-file.tar.gz'
+      end
+    RUBY
+  end
+end

spec/support/expect_violation.rb

@@ -42,7 +42,8 @@ class Expectation
     VIOLATION = :violation
     SOURCE    = :line
 
-    include Adamantium, Concord.new(:string)
+    include Concord.new(:string)
+    include Adamantium
 
     def source
       source_map.to_s
@@ -125,9 +126,9 @@ def self.parse(text:, line_number:)
         )
       end
 
-      include Anima.new(:message, :column_range, :line_number),
-              Adamantium,
-              Comparable
+      include Comparable
+      include Adamantium
+      include Anima.new(:message, :column_range, :line_number)
 
       def <=>(other)
         to_a <=> other.to_a
@@ -142,7 +143,8 @@ def to_a
       class Parser
         COLUMN_PATTERN = /^ *(?<carets>\^\^*) (?<message>.+)$/
 
-        include Concord.new(:text), Adamantium
+        include Adamantium
+        include Concord.new(:text)
 
         def column_range
           Range.new(*match.offset(:carets), true)