From c7f3880d40495c9ce43ed8399ea31d68cf28a2ca Mon Sep 17 00:00:00 2001
From: Hailee Kenney <hailee@puppetlabs.com>
Date: Tue, 7 Oct 2014 16:09:58 -0700
Subject: [PATCH 1/3] (PDOC-8) Add .rubocop.yml to project

Since we want to use rubocop to help maintain the code quality of
this project, add a .rubocop.yml file (borrowed from the puppet repo)
so that we can successfully enforce the things we want and disable
those we don't.
---
 .rubocop.yml | 494 +++++++++++++++++++++++++++++++++++++++++++++++++++
 Gemfile      |   1 +
 2 files changed, 495 insertions(+)
 create mode 100644 .rubocop.yml

diff --git a/.rubocop.yml b/.rubocop.yml
new file mode 100644
index 0000000..8764883
--- /dev/null
+++ b/.rubocop.yml
@@ -0,0 +1,494 @@
+AllCops:
+  Exclude:
+    # Ignore HTML related things
+    - '**/*.erb'
+    - 'lib/puppetx/puppetlabs/strings/yard/templates/**/*'
+
+Lint/ConditionPosition:
+  Enabled: true
+
+Lint/ElseLayout:
+  Enabled: true
+
+Lint/UnreachableCode:
+  Enabled: true
+
+Lint/UselessComparison:
+  Enabled: true
+
+# MAYBE useful - no return inside ensure block.
+Lint/EnsureReturn:
+  Enabled: false
+
+# MAYBE useful - errors when rescue {} happens.
+Lint/HandleExceptions:
+  Enabled: false
+
+# MAYBE useful - catches while 1
+Lint/LiteralInCondition:
+  Enabled: false
+
+# MAYBE useful - but too many instances
+Lint/ShadowingOuterLocalVariable:
+  Enabled: false
+
+# Can catch complicated strings.
+Lint/LiteralInInterpolation:
+  Enabled: true
+
+# DISABLED really useless. Detects return as last statement.
+Style/RedundantReturn:
+  Enabled: false
+
+# DISABLED since the instances do not seem to indicate any specific errors.
+Lint/AmbiguousOperator:
+  Enabled: false
+
+Lint/AssignmentInCondition:
+  Enabled: true
+
+# DISABLED - not useful
+Style/SpaceBeforeComment:
+  Enabled: false
+
+# DISABLED - not useful
+Style/HashSyntax:
+  Enabled: false
+
+# USES: as shortcut for non nil&valid checking a = x() and a.empty?
+# DISABLED - not useful
+Style/AndOr:
+  Enabled: false
+
+# DISABLED - not useful
+Style/RedundantSelf:
+  Enabled: false
+
+# DISABLED - not useful
+Style/MethodLength:
+  Enabled: false
+
+# DISABLED - not useful
+Style/WhileUntilModifier:
+  Enabled: false
+
+# DISABLED - the offender is just haskell envy
+Lint/AmbiguousRegexpLiteral:
+  Enabled: false
+
+# DISABLED
+Lint/Eval:
+  Enabled: false
+
+# DISABLED
+Lint/BlockAlignment:
+  Enabled: false
+
+# DISABLED
+Lint/DefEndAlignment:
+  Enabled: false
+
+# DISABLED
+Lint/EndAlignment:
+  Enabled: false
+
+# DISABLED
+Lint/DeprecatedClassMethods:
+  Enabled: false
+
+# DISABLED
+Lint/Loop:
+  Enabled: false
+
+# DISABLED
+Lint/ParenthesesAsGroupedExpression:
+  Enabled: false
+
+Lint/RescueException:
+  Enabled: false
+
+Lint/StringConversionInInterpolation:
+  Enabled: false
+
+Lint/UnusedBlockArgument:
+  Enabled: false
+
+Lint/UnusedMethodArgument:
+  Enabled: false
+
+Lint/UselessAccessModifier:
+  Enabled: true
+
+Lint/UselessAssignment:
+  Enabled: true
+
+Lint/Void:
+  Enabled: true
+
+Style/AccessModifierIndentation:
+  Enabled: false
+
+Style/AccessorMethodName:
+  Enabled: false
+
+Style/Alias:
+  Enabled: false
+
+Style/AlignArray:
+  Enabled: false
+
+Style/AlignHash:
+  Enabled: false
+
+Style/AlignParameters:
+  Enabled: false
+
+Style/BlockNesting:
+  Enabled: false
+
+Style/AsciiComments:
+  Enabled: false
+
+Style/Attr:
+  Enabled: false
+
+Style/Blocks:
+  Enabled: false
+
+Style/BracesAroundHashParameters:
+  Enabled: false
+
+Style/CaseEquality:
+  Enabled: false
+
+Style/CaseIndentation:
+  Enabled: false
+
+Style/CharacterLiteral:
+  Enabled: false
+
+Style/ClassAndModuleCamelCase:
+  Enabled: false
+
+Style/ClassAndModuleChildren:
+  Enabled: false
+
+Style/ClassCheck:
+  Enabled: false
+
+Style/ClassLength:
+  Enabled: false
+
+Style/ClassMethods:
+  Enabled: false
+
+Style/ClassVars:
+  Enabled: false
+
+Style/WhenThen:
+  Enabled: false
+
+# DISABLED - not useful
+Style/WordArray:
+  Enabled: false
+
+Style/UnneededPercentQ:
+  Enabled: false
+
+Style/Tab:
+  Enabled: false
+
+Style/SpaceBeforeSemicolon:
+  Enabled: false
+
+Style/TrailingBlankLines:
+  Enabled: false
+
+Style/SpaceInsideBlockBraces:
+  Enabled: false
+
+Style/SpaceInsideBrackets:
+  Enabled: false
+
+Style/SpaceInsideHashLiteralBraces:
+  Enabled: false
+
+Style/SpaceInsideParens:
+  Enabled: false
+
+Style/LeadingCommentSpace:
+  Enabled: false
+
+Style/SingleSpaceBeforeFirstArg:
+  Enabled: false
+
+Style/SpaceAfterColon:
+  Enabled: false
+
+Style/SpaceAfterComma:
+  Enabled: false
+
+Style/SpaceAfterControlKeyword:
+  Enabled: false
+
+Style/SpaceAfterMethodName:
+  Enabled: false
+
+Style/SpaceAfterNot:
+  Enabled: false
+
+Style/SpaceAfterSemicolon:
+  Enabled: false
+
+Style/SpaceAroundEqualsInParameterDefault:
+  Enabled: false
+
+Style/SpaceAroundOperators:
+  Enabled: false
+
+Style/SpaceBeforeBlockBraces:
+  Enabled: false
+
+Style/SpaceBeforeComma:
+  Enabled: false
+
+Style/CollectionMethods:
+  Enabled: false
+
+Style/CommentIndentation:
+  Enabled: false
+
+Style/ColonMethodCall:
+  Enabled: false
+
+Style/CommentAnnotation:
+  Enabled: false
+
+Style/CyclomaticComplexity:
+  Enabled: false
+
+Style/ConstantName:
+  Enabled: false
+
+Style/Documentation:
+  Enabled: false
+
+Style/DefWithParentheses:
+  Enabled: false
+
+Style/DeprecatedHashMethods:
+  Enabled: false
+
+Style/DotPosition:
+  Enabled: false
+
+# DISABLED - used for converting to bool
+Style/DoubleNegation:
+  Enabled: false
+
+Style/EachWithObject:
+  Enabled: false
+
+Style/EmptyLineBetweenDefs:
+  Enabled: false
+
+Style/IndentArray:
+  Enabled: false
+
+Style/IndentHash:
+  Enabled: false
+
+Style/IndentationConsistency:
+  Enabled: false
+
+Style/IndentationWidth:
+  Enabled: false
+
+Style/EmptyLines:
+  Enabled: false
+
+Style/EmptyLinesAroundAccessModifier:
+  Enabled: false
+
+Style/EmptyLinesAroundBody:
+  Enabled: false
+
+Style/EmptyLiteral:
+  Enabled: false
+
+Style/LineLength:
+  Enabled: false
+
+Style/MethodCallParentheses:
+  Enabled: false
+
+Style/MethodDefParentheses:
+  Enabled: false
+
+Style/LineEndConcatenation:
+  Enabled: false
+
+Style/TrailingWhitespace:
+  Enabled: false
+
+Style/StringLiterals:
+  Enabled: false
+
+Style/TrailingComma:
+  Enabled: false
+
+Style/GlobalVars:
+  Enabled: false
+
+Style/GuardClause:
+  Enabled: false
+
+Style/IfUnlessModifier:
+  Enabled: false
+
+Style/MultilineIfThen:
+  Enabled: false
+
+Style/NegatedIf:
+  Enabled: false
+
+Style/NegatedWhile:
+  Enabled: false
+
+Style/Next:
+  Enabled: false
+
+Style/SingleLineBlockParams:
+  Enabled: false
+
+Style/SingleLineMethods:
+  Enabled: false
+
+Style/SpecialGlobalVars:
+  Enabled: false
+
+Style/TrivialAccessors:
+  Enabled: false
+
+Style/UnlessElse:
+  Enabled: false
+
+Style/UnneededPercentX:
+  Enabled: false
+
+Style/VariableInterpolation:
+  Enabled: false
+
+Style/VariableName:
+  Enabled: false
+
+Style/WhileUntilDo:
+  Enabled: false
+
+Style/EvenOdd:
+  Enabled: false
+
+Style/FileName:
+  Enabled: false
+
+Style/For:
+  Enabled: false
+
+Style/Lambda:
+  Enabled: false
+
+Style/MethodName:
+  Enabled: false
+
+Style/MultilineTernaryOperator:
+  Enabled: false
+
+Style/NestedTernaryOperator:
+  Enabled: false
+
+Style/NilComparison:
+  Enabled: false
+
+Style/FormatString:
+  Enabled: false
+
+Style/MultilineBlockChain:
+  Enabled: false
+
+Style/Semicolon:
+  Enabled: false
+
+Style/SignalException:
+  Enabled: false
+
+Style/NonNilCheck:
+  Enabled: false
+
+Style/Not:
+  Enabled: false
+
+Style/NumericLiterals:
+  Enabled: false
+
+Style/OneLineConditional:
+  Enabled: false
+
+Style/OpMethod:
+  Enabled: false
+
+Style/ParenthesesAroundCondition:
+  Enabled: false
+
+Style/PercentLiteralDelimiters:
+  Enabled: false
+
+Style/PerlBackrefs:
+  Enabled: false
+
+Style/PredicateName:
+  Enabled: false
+
+Style/RedundantException:
+  Enabled: false
+
+Style/SelfAssignment:
+  Enabled: false
+
+Style/Proc:
+  Enabled: false
+
+Style/RaiseArgs:
+  Enabled: false
+
+Style/RedundantBegin:
+  Enabled: false
+
+Style/RescueModifier:
+  Enabled: false
+
+Style/RegexpLiteral:
+  Enabled: false
+
+Lint/UnderscorePrefixedVariableName:
+  Enabled: false
+
+Style/ParameterLists:
+  Enabled: false
+
+Lint/RequireParentheses:
+  Enabled: false
+
+Lint/SpaceBeforeFirstArg:
+  Enabled: false
+
+Style/ModuleFunction:
+  Enabled: false
+
+Lint/Debugger:
+  Enabled: false
+
+Style/IfWithSemicolon:
+  Enabled: false
+
+Style/Encoding:
+  Enabled: false
diff --git a/Gemfile b/Gemfile
index dfb1ad6..b1500df 100644
--- a/Gemfile
+++ b/Gemfile
@@ -24,4 +24,5 @@ end
 group :development do
   gem 'pry'
   gem 'pry-debugger'
+  gem 'rubocop'
 end

From 999daa9c4c3e14ca6cec4ba3952b9675752eb513 Mon Sep 17 00:00:00 2001
From: Hailee Kenney <hailee@puppetlabs.com>
Date: Wed, 8 Oct 2014 11:36:21 -0700
Subject: [PATCH 2/3] (PDOC-8) Fix present rubocop offenses

Since we want to run rubocop in travis when pull requests are
submitted, fix a few pre-existing rubocop offenses.
---
 Gemfile                        | 4 +++-
 lib/puppet/face/strings.rb     | 6 ++++--
 spec/spec_helper_acceptance.rb | 2 --
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/Gemfile b/Gemfile
index b1500df..6825907 100644
--- a/Gemfile
+++ b/Gemfile
@@ -5,7 +5,9 @@ gem 'rgen'
 gem 'redcarpet'
 gem 'puppet-strings', '0.1.0', :path => '.'
 
-if puppetversion = ENV['PUPPET_VERSION']
+puppetversion = ENV['PUPPET_VERSION']
+
+if puppetversion
   gem 'puppet', puppetversion
 else
   gem 'puppet', '~> 3.6.2'
diff --git a/lib/puppet/face/strings.rb b/lib/puppet/face/strings.rb
index 200ff35..4b24bee 100644
--- a/lib/puppet/face/strings.rb
+++ b/lib/puppet/face/strings.rb
@@ -38,12 +38,14 @@ Puppet::Face.define(:strings, '0.0.1') do
       yardoc_actions = Puppetx::PuppetLabs::Strings::Actions.new(Puppet[:debug], Puppet[:trace])
 
       # The last element of the argument array should be the options hash.
+      # We don't have any options yet, so for now just pop the hash off and
+      # toss it.
       #
       # NOTE: The Puppet Face will throw 'unrecognized option' errors if any
       # YARD options are passed to it. The best way to approach this problem is
       # by using the `.yardopts` file. YARD will autoload any options placed in
       # that file.
-      opts = args.pop
+      args.pop
 
       # For now, assume the remaining positional args are a list of manifest
       # and ruby files to parse.
@@ -67,7 +69,7 @@ Puppet::Face.define(:strings, '0.0.1') do
 
       server_actions = Puppetx::PuppetLabs::Strings::Actions.new(Puppet[:debug], Puppet[:trace])
 
-      opts = args.pop
+      args.pop
 
       module_names = args
 
diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb
index 52f5d83..5b230d3 100644
--- a/spec/spec_helper_acceptance.rb
+++ b/spec/spec_helper_acceptance.rb
@@ -6,8 +6,6 @@ unless ENV['RS_PROVISION'] == 'no'
 end
 
 RSpec.configure do |c|
-  # Project root
-  proj_root = File.expand_path(File.join(File.dirname(__FILE__), '..'))
 
   # Readable test descriptions
   c.formatter = :documentation

From 1abca4705f876e62ba0439710003cfd8c0ee155b Mon Sep 17 00:00:00 2001
From: Hailee Kenney <hailee@puppetlabs.com>
Date: Wed, 8 Oct 2014 13:35:14 -0700
Subject: [PATCH 3/3] (PDOC-8) Add rubocop to travis job

In order to ensure long term code quality, add a rubocop check to
the travis job which runs when a pull request is submitted. Do this
by adding a rake task for rubocop and setting up travis to run
both the spec rake task and the rubocop one.

Additionally, make a few changes to the .rubocop.yml file so it
will stop complaining about incorrect namespaces.
---
 .rubocop.yml | 12 ++++++------
 .travis.yml  | 12 ++++++------
 Gemfile      |  2 +-
 Rakefile     |  6 ++++++
 4 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/.rubocop.yml b/.rubocop.yml
index 8764883..8466b96 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -65,7 +65,7 @@ Style/RedundantSelf:
   Enabled: false
 
 # DISABLED - not useful
-Style/MethodLength:
+Metrics/MethodLength:
   Enabled: false
 
 # DISABLED - not useful
@@ -143,7 +143,7 @@ Style/AlignHash:
 Style/AlignParameters:
   Enabled: false
 
-Style/BlockNesting:
+Metrics/BlockNesting:
   Enabled: false
 
 Style/AsciiComments:
@@ -176,7 +176,7 @@ Style/ClassAndModuleChildren:
 Style/ClassCheck:
   Enabled: false
 
-Style/ClassLength:
+Metrics/ClassLength:
   Enabled: false
 
 Style/ClassMethods:
@@ -264,7 +264,7 @@ Style/ColonMethodCall:
 Style/CommentAnnotation:
   Enabled: false
 
-Style/CyclomaticComplexity:
+Metrics/CyclomaticComplexity:
   Enabled: false
 
 Style/ConstantName:
@@ -316,7 +316,7 @@ Style/EmptyLinesAroundBody:
 Style/EmptyLiteral:
   Enabled: false
 
-Style/LineLength:
+Metrics/LineLength:
   Enabled: false
 
 Style/MethodCallParentheses:
@@ -472,7 +472,7 @@ Style/RegexpLiteral:
 Lint/UnderscorePrefixedVariableName:
   Enabled: false
 
-Style/ParameterLists:
+Metrics/ParameterLists:
   Enabled: false
 
 Lint/RequireParentheses:
diff --git a/.travis.yml b/.travis.yml
index f73e076..a6204b6 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,17 +1,17 @@
 language: ruby
 bundler_args: --without development
-script: "bundle exec rake spec SPEC_OPTS='--color --format documentation'"
+script:
+  - "bundle exec rake $CHECK"
 notifications:
   email: false
 rvm:
   - 1.9.3
   - 2.0.0
   - 2.1.0
-  - ruby-head
+
 env:
-  - PUPPET_VERSION=3.6.2
-  - PUPPET_VERSION=3.7.1
+  - "CHECK=spec SPEC_OPTS='--color --format documentation'"
+  - "CHECK=rubocop"
+
 matrix:
   fast_finish: true
-  allow_failures:
-    - rvm: ruby-head
diff --git a/Gemfile b/Gemfile
index 6825907..b37a722 100644
--- a/Gemfile
+++ b/Gemfile
@@ -21,10 +21,10 @@ group :test do
   gem 'serverspec'
   gem 'beaker'
   gem 'beaker-rspec'
+  gem 'rubocop'
 end
 
 group :development do
   gem 'pry'
   gem 'pry-debugger'
-  gem 'rubocop'
 end
diff --git a/Rakefile b/Rakefile
index e648b48..3bda0b1 100644
--- a/Rakefile
+++ b/Rakefile
@@ -22,3 +22,9 @@ task :acceptance do
   sh "puppet module build spec/unit/puppet/examples/test"
   sh "BEAKER_set=#{ENV["platform"]} rspec spec/acceptance/*.rb"
 end
+
+task(:rubocop) do
+  require 'rubocop'
+  cli = RuboCop::CLI.new
+  cli.run(%w(-D -f s))
+end