Merge pull request #127 from cedef/misc/lint

Update Gemfile, rubocop and cosmetic lint tools

.gitignore

@@ -1,6 +1,8 @@
 pkg/
 .*.sw?
-spec/fixtures
+Gemfile.lock
+spec/fixtures/manifests
+spec/fixtures/modules
 Gemfile.lock
 .rspec_system
 .vagrant

.rspec

@@ -0,0 +1,4 @@
+--format documentation
+--color
+
+

.rubocop.yml

@@ -0,0 +1,484 @@
+AllCops:
+  Exclude:
+    # Ignore HTML related things
+    - '**/*.erb'
+    # Ignore vendored gems
+    - 'vendor/**/*'
+    # Ignore code from test fixtures
+    - 'spec/fixtures/**/*'
+    # Ignore lib/** as it contains a lot of offenses for now.
+    # TODO : fix lib/** offenses
+    - 'lib/**/*'
+
+Lint/ConditionPosition:
+  Enabled: true
+
+Lint/ElseLayout:
+  Enabled: true
+
+Lint/UnreachableCode:
+  Enabled: true
+
+Lint/UselessComparison:
+  Enabled: true
+
+Lint/EnsureReturn:
+  Enabled: true
+
+Lint/HandleExceptions:
+  Enabled: true
+
+Lint/LiteralInCondition:
+  Enabled: true
+
+Lint/ShadowingOuterLocalVariable:
+  Enabled: true
+
+Lint/LiteralInInterpolation:
+  Enabled: true
+
+Style/RedundantReturn:
+  Enabled: true
+
+Lint/AmbiguousOperator:
+  Enabled: true
+
+Lint/AssignmentInCondition:
+  Enabled: true
+
+Style/SpaceBeforeComment:
+  Enabled: true
+
+# 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
+Metrics/MethodLength:
+  Enabled: false
+
+# DISABLED
+Metrics/BlockLength:
+  Enabled: false
+
+# DISABLED - not useful
+Style/WhileUntilModifier:
+  Enabled: false
+
+# DISABLED - the offender is just haskell envy
+Lint/AmbiguousRegexpLiteral:
+  Enabled: false
+
+# DISABLED
+Security/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
+
+Metrics/BlockNesting:
+  Enabled: false
+
+Style/AsciiComments:
+  Enabled: false
+
+Style/Attr:
+  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
+
+Metrics/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/SpaceAfterColon:
+  Enabled: false
+
+Style/SpaceAfterComma:
+  Enabled: false
+
+Style/SpaceAroundKeyword:
+  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
+
+Metrics/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/EmptyLiteral:
+  Enabled: false
+
+Metrics/LineLength:
+  Enabled: false
+
+Style/MethodCallWithoutArgsParentheses:
+  Enabled: false
+
+Style/MethodDefParentheses:
+  Enabled: false
+
+Style/LineEndConcatenation:
+  Enabled: false
+
+Style/TrailingWhitespace:
+  Enabled: false
+
+Style/StringLiterals:
+  Enabled: false
+
+Style/TrailingCommaInLiteral:
+  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/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
+
+Metrics/ParameterLists:
+  Enabled: false
+
+Lint/RequireParentheses:
+  Enabled: false
+
+Style/SpaceBeforeFirstArg:
+  Enabled: false
+
+Style/ModuleFunction:
+  Enabled: false
+
+Lint/Debugger:
+  Enabled: false
+
+Style/IfWithSemicolon:
+  Enabled: false
+
+Style/Encoding:
+  Enabled: false

.travis.yml

@@ -1,12 +1,19 @@
+---
 language: ruby
-rvm:
+sudo: false
-  - 2.0.0
+bundler_args: --without development system_tests
-  - 2.1.0
+before_install: rm Gemfile.lock || true
+script: bundle exec rake test
 env:
-  - PUPPET_VERSION=4.0.0
+  - PUPPET_GEM_VERSION="~> 4.0.0"
-  - PUPPET_VERSION=4.5.0
+  - PUPPET_GEM_VERSION="~> 4.5.0"
-gemfile: Gemfile.travis
+  - PUPPET_GEM_VERSION="~> 4.9.0"
-before_install: 'cp Gemfile Gemfile.travis'
+rvm:
-notifications:
+  - 2.2
-  email:
+matrix:
-    - 'inkblot@movealong.org'
+  exclude:
+    - rvm: 2.2
+      env: PUPPET_GEM_VERSION="~> 3.7.0"
+    - rvm: 2.2
+      env: PUPPET_GEM_VERSION="~> 3.8.0"
+  fast_finish: false

Gemfile

@@ -1,14 +1,40 @@
-source 'https://rubygems.org'
+source ENV['GEM_SOURCE'] || "https://rubygems.org"
 
-if ENV.include?('PUPPET_VERSION')
+group :test do
-  puppetversion = "~>#{ENV['PUPPET_VERSION']}"
+  puppetversion = ENV.key?('PUPPET_VERSION') ? "= #{ENV['PUPPET_VERSION']}" : ['>= 4.9']
-else
+  gem "metadata-json-lint"
-  puppetversion = '~>4.5.0'
+  gem "puppet", puppetversion
+  gem "puppetlabs_spec_helper"
+  gem "rake"
+  gem "rspec", '> 3.4.0'
+  gem "rspec-puppet"
+  gem "rspec-puppet-facts"
+  gem 'rubocop'
+  gem 'simplecov', '>= 0.11.0'
+  gem 'simplecov-console'
+
+  gem "puppet-lint-absolute_classname-check"
+  gem "puppet-lint-classes_and_types_beginning_with_digits-check"
+  gem "puppet-lint-leading_zero-check"
+  gem 'puppet-lint-resource_reference_syntax'
+  gem "puppet-lint-trailing_comma-check"
+  gem "puppet-lint-unquoted_string-check"
+  gem "puppet-lint-version_comparison-check"
+
+  gem 'json_pure', '<= 2.0.1' if RUBY_VERSION < '2.0.0'
 end
 
-gem 'rake'
+group :development do
-gem 'puppet', puppetversion
+  gem "guard-rake" if RUBY_VERSION >= '2.2.5' # per dependency https://rubygems.org/gems/ruby_dep
-gem 'puppet-lint'
+  gem "puppet-blacksmith"
-gem 'rspec-puppet'
+  gem 'puppet-strings'
-gem 'puppetlabs_spec_helper'
+  gem "travis" if RUBY_VERSION >= '2.1.0'
-gem 'metadata-json-lint'
+  gem "travis-lint" if RUBY_VERSION >= '2.1.0'
+  gem 'yard'
+end
+
+group :system_tests do
+  gem "beaker"
+  gem "beaker-puppet_install_helper"
+  gem "beaker-rspec"
+end

Gemfile.lock

@@ -1,57 +0,0 @@
-GEM
-  remote: https://rubygems.org/
-  specs:
-    CFPropertyList (2.2.8)
-    diff-lcs (1.2.5)
-    facter (2.4.6)
-      CFPropertyList (~> 2.2.6)
-    hiera (3.2.1)
-    json_pure (2.0.2)
-    metaclass (0.0.4)
-    metadata-json-lint (0.0.2)
-    mocha (1.1.0)
-      metaclass (~> 0.0.1)
-    puppet (4.5.3)
-      CFPropertyList (~> 2.2.6)
-      facter (> 2.0, < 4)
-      hiera (>= 2.0, < 4)
-      json_pure
-    puppet-lint (1.1.0)
-    puppet-syntax (1.3.0)
-      rake
-    puppetlabs_spec_helper (0.8.2)
-      mocha
-      puppet-lint
-      puppet-syntax
-      rake
-      rspec
-      rspec-puppet
-    rake (10.4.2)
-    rspec (3.1.0)
-      rspec-core (~> 3.1.0)
-      rspec-expectations (~> 3.1.0)
-      rspec-mocks (~> 3.1.0)
-    rspec-core (3.1.7)
-      rspec-support (~> 3.1.0)
-    rspec-expectations (3.1.2)
-      diff-lcs (>= 1.2.0, < 2.0)
-      rspec-support (~> 3.1.0)
-    rspec-mocks (3.1.3)
-      rspec-support (~> 3.1.0)
-    rspec-puppet (1.0.1)
-      rspec
-    rspec-support (3.1.2)
-
-PLATFORMS
-  ruby
-
-DEPENDENCIES
-  metadata-json-lint
-  puppet (~> 4.5.0)
-  puppet-lint
-  puppetlabs_spec_helper
-  rake
-  rspec-puppet
-
-BUNDLED WITH
-   1.13.5

Rakefile

@@ -1,40 +1,62 @@
+require 'rubygems'
+require 'bundler/setup'
+
 require 'puppetlabs_spec_helper/rake_tasks'
+require 'puppet/version'
 require 'puppet-lint/tasks/puppet-lint'
 require 'puppet-syntax/tasks/puppet-syntax'
+require 'metadata-json-lint/rake_task'
+require 'rubocop/rake_task'
+
+if Puppet.version.to_f >= 4.9
+    require 'semantic_puppet'
+elsif Puppet.version.to_f >= 3.6 && Puppet.version.to_f < 4.9
+    require 'puppet/vendor/semantic/lib/semantic'
+end
+
+# These gems aren't always present, for instance
+# on Travis with --without development
+begin
+  require 'puppet_blacksmith/rake_tasks'
+rescue LoadError # rubocop:disable Lint/HandleExceptions
+end
+
+#RuboCop::RakeTask.new
 
 exclude_paths = [
-  'bundle/**/*',
+  "bundle/**/*",
-  'pkg/**/*',
+  "pkg/**/*",
-  'vendor/**/*',
+  "vendor/**/*",
-  'spec/**/*'
+  "spec/**/*",
 ]
 
+# Coverage from puppetlabs-spec-helper requires rcov which
+# doesn't work in anything since 1.8.7
+#Rake::Task[:coverage].clear
+
 Rake::Task[:lint].clear
+
+PuppetLint.configuration.relative = true
+PuppetLint.configuration.disable_80chars
+PuppetLint.configuration.disable_class_inherits_from_params_class
+PuppetLint.configuration.disable_class_parameter_defaults
+PuppetLint.configuration.fail_on_warnings = true
+
 PuppetLint::RakeTask.new :lint do |config|
-  config.fail_on_warnings = true
   config.ignore_paths = exclude_paths
-  config.disable_checks = [
-    '80chars',
-    'class_parameter_defaults',
-    'documentation',
-    'autoloader_layout'
-  ]
 end
 
 PuppetSyntax.exclude_paths = exclude_paths
 
+desc "Run acceptance tests"
 RSpec::Core::RakeTask.new(:acceptance) do |t|
   t.pattern = 'spec/acceptance'
 end
 
-task :metadata do
+desc "Populate CONTRIBUTORS file"
-  sh 'metadata-json-lint metadata.json'
+task :contributors do
+  system("git log --format='%aN' | sort -u > CONTRIBUTORS")
 end
 
-desc 'Run syntax, lint, and spec tests.'
+desc "Run syntax, lint, and spec tests."
-task :test => [
+task :test => %i(metadata_lint syntax lint rubocop spec)
-  :syntax,
-  :lint,
-  :spec,
-  :metadata
-]

manifests/init.pp

@@ -15,7 +15,7 @@ class bind (
     File {
         ensure  => present,
         owner   => 'root',
-        group   => $bind_group,
+        group   => $::bind::defaults::bind_group,
         mode    => '0644',
         require => Package['bind'],
         notify  => Service['bind'],
@@ -25,7 +25,7 @@ class bind (
 
     package { 'bind':
         ensure => latest,
-        name   => $bind_package,
+        name   => $::bind::defaults::bind_package,
     }
 
     if $dnssec {
@@ -42,7 +42,7 @@ class bind (
     bind::key { 'rndc-key':
         algorithm   => 'hmac-md5',
         secret_bits => '512',
-        keydir      => $confdir,
+        keydir      => $bind::defaults::confdir,
         keyfile     => 'rndc.key',
         include     => false,
     }
@@ -55,36 +55,36 @@ class bind (
         content => template('bind/rndc-helper.erb'),
     }
 
-    file { "${confdir}/zones":
+    file { "${::bind::defaults::confdir}/zones":
-        ensure  => directory,
+        ensure => directory,
-        mode    => '2755',
+        mode   => '2755',
     }
 
-    file { $namedconf:
+    file { $::bind::defaults::namedconf:
         content => template('bind/named.conf.erb'),
     }
 
-    if $include_default_zones and $default_zones_source {
+    if $include_default_zones and $::bind::defaults::default_zones_source {
-        file { $default_zones_include:
+        file { $::bind::defaults::default_zones_include:
-            source => $default_zones_source,
+            source => $::bind::defaults::default_zones_source,
         }
     }
 
-    class { 'bind::keydir':
+    class { '::bind::keydir':
-        keydir => "${confdir}/keys",
+        keydir => "${::bind::defaults::confdir}/keys",
     }
 
     concat { [
-        "${confdir}/acls.conf",
+        "${::bind::defaults::confdir}/acls.conf",
-        "${confdir}/keys.conf",
+        "${::bind::defaults::confdir}/keys.conf",
-        "${confdir}/views.conf",
+        "${::bind::defaults::confdir}/views.conf",
-        "${confdir}/servers.conf",
+        "${::bind::defaults::confdir}/servers.conf",
-        "${confdir}/logging.conf",
+        "${::bind::defaults::confdir}/logging.conf",
-        "${confdir}/view-mappings.txt",
+        "${::bind::defaults::confdir}/view-mappings.txt",
-        "${confdir}/domain-mappings.txt",
+        "${::bind::defaults::confdir}/domain-mappings.txt",
         ]:
         owner   => 'root',
-        group   => $bind_group,
+        group   => $::bind::defaults::bind_group,
         mode    => '0644',
         warn    => true,
         require => Package['bind'],
@@ -92,20 +92,20 @@ class bind (
     }
 
     concat::fragment { 'bind-logging-header':
-        order   => "00-header",
+        order   => '00-header',
-        target  => "${confdir}/logging.conf",
+        target  => "${::bind::defaults::confdir}/logging.conf",
         content => "logging {\n";
     }
 
     concat::fragment { 'bind-logging-footer':
-        order   => "99-footer",
+        order   => '99-footer',
-        target  => "${confdir}/logging.conf",
+        target  => "${::bind::defaults::confdir}/logging.conf",
         content => "};\n";
     }
 
     service { 'bind':
         ensure     => running,
-        name       => $bind_service,
+        name       => $::bind::defaults::bind_service,
         enable     => true,
         hasrestart => true,
         hasstatus  => true,

manifests/logging/category.pp

@@ -1,7 +1,7 @@
 # ex: syntax=puppet si ts=4 sw=4 et
 
 define bind::logging::category (
-	$channels
+    $channels
 ) {
     concat::fragment { "bind-logging-category-${name}":
         order   => "60-${name}",

manifests/server.pp

@@ -6,7 +6,7 @@ define bind::server (
     $key       = undef,
     $transfers = undef,
 ) {
-    include bind
+    include ::bind
 
     concat::fragment { "bind-server-${name}":
         order   => 10,

manifests/updater.pp

@@ -4,10 +4,10 @@ class bind::updater (
     $keydir = undef,
 ) inherits bind::defaults {
 
-    if $nsupdate_package {
+    if $::bind::defaults::nsupdate_package {
         package { 'bind-tools':
             ensure => present,
-            name   => $nsupdate_package,
+            name   => $::bind::defaults::nsupdate_package,
         }
     }
 

manifests/zone.pp

@@ -22,7 +22,7 @@ define bind::zone (
     $forwarders_port = 53,
 ) {
     # where there is a zone, there is a server
-    include bind
+    include ::bind
 
     # Pull some platform defaults and `bind` class parameters into the local scope
     $cachedir = $::bind::defaults::cachedir
@@ -54,15 +54,15 @@ define bind::zone (
     }
 
     unless !($allow_updates != '' and ! $dynamic) {
-        fail("allow_updates may only be provided for bind::zone resources with dynamic set to true")
+        fail('allow_updates may only be provided for bind::zone resources with dynamic set to true')
     }
 
     unless !($dnssec and ! $dynamic) {
-        fail("dnssec may only be true for bind::zone resources with dynamic set to true")
+        fail('dnssec may only be true for bind::zone resources with dynamic set to true')
     }
 
     unless !($key_directory != '' and ! $dnssec) {
-        fail("key_directory may only be provided for bind::zone resources with dnssec set to true")
+        fail('key_directory may only be provided for bind::zone resources with dnssec set to true')
     }
 
     unless !($allow_notify != '' and ! member(['slave', 'stub'], $zone_type)) {

metadata.json

@@ -27,8 +27,8 @@
     }
   ],
   "dependencies": [
-    { "name": "puppetlabs/stdlib" },
+    { "name": "puppetlabs/stdlib", "version_requirement": ">=4.15 < 5.0.0" },
-    { "name": "puppetlabs/concat", "version_requirement": ">=2.0.0" }
+    { "name": "puppetlabs/concat", "version_requirement": ">=2.2.1 < 3.0.0" }
   ],
   "data_provider": "hiera"
 }