Merge pull request #199 from sul-dlss/more-rubocop-fixes

More rubocop fixes

.gitignore

@@ -2,6 +2,7 @@
 .DS_Store
 .bundle
 .idea
+.pry_history
 .ruby-gemset
 .ruby-version
 .rvmrc

.rubocop.yml

@@ -18,8 +18,13 @@ Style/EmptyLineBetweenDefs:
     - lib/dor/models/editable.rb
     - lib/dor/models/governable.rb
 
-# Configuration parameters: AllowURI, URISchemes.
 Metrics/LineLength:
   Max: 200
   Exclude:
+    # rights statement is a single string
     - spec/dor/editable_spec.rb
+
+# test is specifically written this way
+Lint/LiteralInCondition:
+  Exclude:
+    - 'spec/dor/upgradable_spec.rb'

.rubocop_todo.yml

@@ -1,41 +1,27 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2016-03-10 14:52:35 -0800 using RuboCop version 0.37.2.
+# on 2016-03-10 17:38:42 -0800 using RuboCop version 0.37.2.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 6
+# Offense count: 5
 Lint/AmbiguousOperator:
   Exclude:
     - 'lib/dor-services.rb'
     - 'lib/dor/config.rb'
-    - 'lib/dor/datastreams/workflow_ds.rb'
     - 'lib/dor/models/describable.rb'
     - 'lib/dor/models/eventable.rb'
     - 'lib/dor/models/workflow_object.rb'
 
-# Offense count: 2
-Lint/AmbiguousRegexpLiteral:
-  Exclude:
-    - 'script/manual_integration.rb'
-    - 'spec/dor/processable_spec.rb'
-
 # Offense count: 2
 # Configuration parameters: AllowSafeAssignment.
 Lint/AssignmentInCondition:
   Exclude:
     - 'lib/dor/datastreams/workflow_definition_ds.rb'
     - 'lib/dor/utils/predicate_patch.rb'
 
-# Offense count: 2
-# Cop supports --auto-correct.
-# Configuration parameters: AlignWith, SupportedStyles, AutoCorrect.
-# SupportedStyles: keyword, variable, start_of_line
-Lint/EndAlignment:
-  Enabled: false
-
 # Offense count: 6
 Lint/HandleExceptions:
   Exclude:
@@ -45,11 +31,6 @@ Lint/HandleExceptions:
     - 'script/manual_integration.rb'
     - 'spec/foxml_helper.rb'
 
-# Offense count: 1
-Lint/LiteralInCondition:
-  Exclude:
-    - 'spec/dor/upgradable_spec.rb'
-
 # Offense count: 4
 Lint/RescueException:
   Exclude:
@@ -58,11 +39,6 @@ Lint/RescueException:
     - 'lib/dor/services/indexing_service.rb'
     - 'lib/dor/services/sdr_ingest_service.rb'
 
-# Offense count: 1
-Lint/ShadowingOuterLocalVariable:
-  Exclude:
-    - 'lib/dor/datastreams/default_object_rights_ds.rb'
-
 # Offense count: 22
 # Cop supports --auto-correct.
 # Configuration parameters: IgnoreEmptyBlocks.
@@ -130,25 +106,12 @@ Metrics/ParameterLists:
 Metrics/PerceivedComplexity:
   Max: 24
 
-# Offense count: 4
-# Cop supports --auto-correct.
-Performance/Casecmp:
-  Exclude:
-    - 'lib/dor/models/identifiable.rb'
-    - 'lib/dor/models/releaseable.rb'
-
 # Offense count: 2
 # Cop supports --auto-correct.
 Performance/RedundantBlockCall:
   Exclude:
     - 'lib/dor/datastreams/events_ds.rb'
 
-# Offense count: 1
-# Cop supports --auto-correct.
-Performance/RedundantMatch:
-  Exclude:
-    - 'lib/dor/datastreams/content_metadata_ds.rb'
-
 # Offense count: 9
 Style/AccessorMethodName:
   Exclude:
@@ -199,7 +162,7 @@ Style/BlockDelimiters:
 Style/BracesAroundHashParameters:
   Enabled: false
 
-# Offense count: 11
+# Offense count: 7
 # Cop supports --auto-correct.
 # Configuration parameters: IndentWhenRelativeTo, SupportedStyles, IndentOneStep, IndentationWidth.
 # SupportedStyles: case, end
@@ -570,7 +533,7 @@ Style/SpaceBeforeSemicolon:
     - 'lib/dor/workflow/process.rb'
     - 'spec/foxml_helper.rb'
 
-# Offense count: 188
+# Offense count: 187
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle, SupportedStyles, EnforcedStyleForEmptyBraces, SpaceBeforeBlockParameters.
 # SupportedStyles: space, no_space
@@ -591,7 +554,7 @@ Style/SpaceInsideBrackets:
     - 'spec/dor/digital_stacks_service_spec.rb'
     - 'spec/dor/processable_spec.rb'
 
-# Offense count: 211
+# Offense count: 208
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle, EnforcedStyleForEmptyBraces, SupportedStyles.
 # SupportedStyles: space, no_space
@@ -632,12 +595,6 @@ Style/SymbolProc:
     - 'lib/dor/utils/ng_tidy.rb'
     - 'spec/dor/describable_spec.rb'
 
-# Offense count: 1
-# Cop supports --auto-correct.
-Style/TrailingWhitespace:
-  Exclude:
-    - 'lib/dor/datastreams/content_metadata_ds.rb'
-
 # Offense count: 8
 # Cop supports --auto-correct.
 Style/UnneededInterpolation:

lib/dor/datastreams/content_metadata_ds.rb

@@ -69,7 +69,7 @@ def public_xml
         externalFile.add_previous_sibling(src_label)
         externalFile << src_image_data unless src_image_data.nil?
       end
-      
+
       result
     end
 
@@ -132,7 +132,7 @@ def to_solr(solr_doc = {}, *args)
           preserved_size += file['size'].to_i if file['preserve'] == 'yes'
           next unless file['shelve'] == 'yes'
           counts['shelved_file'] += 1
-          first_shelved_image ||= file['id'] if file['id'].match(/jp2$/)
+          first_shelved_image ||= file['id'] if file['id'] =~ /jp2$/
         end
       end
       solr_doc['content_type_ssim'              ] = doc.root['type']

lib/dor/datastreams/default_object_rights_ds.rb

@@ -119,7 +119,7 @@ def normalize!
         # cleanup ordering is important here
         doc.xpath('//machine/text()').each { |node| node.content = node.content.strip }
         doc.xpath('//human')
-          .tap { |nodeset| norm.clean_linefeeds(nodeset) }
+          .tap { |node_set| norm.clean_linefeeds(node_set) }
           .each do |node|
             norm.trim_text(node)
             norm.remove_empty_nodes(node)

lib/dor/datastreams/workflow_ds.rb

@@ -64,7 +64,7 @@ def graph(dir = nil)
     #
     # @return [Integer] value of the priority.  Defaults to 0 if none of the workflows are expedited
     def current_priority
-      cp = workflows.detect &:expedited?
+      cp = workflows.detect(&:expedited?)
       return 0 if cp.nil?
       cp.priority.to_i
     end

lib/dor/models/governable.rb

@@ -35,23 +35,25 @@ def set_read_rights(rights)
     end
 
     def add_collection(collection_or_druid)
-      collection = case collection_or_druid
+      collection =
+        case collection_or_druid
         when String
           Dor::Collection.find(collection_or_druid)
         when Dor::Collection
           collection_or_druid
-      end
+        end
       collections << collection
       sets << collection
     end
 
     def remove_collection(collection_or_druid)
-      collection = case collection_or_druid
+      collection =
+        case collection_or_druid
         when String
           Dor::Collection.find(collection_or_druid)
         when Dor::Collection
           collection_or_druid
-      end
+        end
 
       collections.delete(collection)
       sets.delete(collection)

lib/dor/models/identifiable.rb

@@ -152,7 +152,7 @@ def validate_and_normalize_tag(tag_str, existing_tag_list)
       # note that the comparison for duplicate tags is case-insensitive, but we don't change case as part of the normalized version
       # we return, because we want to preserve the user's intended case.
       normalized_tag = normalize_tag_arr(tag_arr)
-      dupe_existing_tag = existing_tag_list.detect { |existing_tag| normalize_tag(existing_tag).downcase == normalized_tag.downcase }
+      dupe_existing_tag = existing_tag_list.detect { |existing_tag| normalize_tag(existing_tag).casecmp(normalized_tag) == 0 }
       if dupe_existing_tag
         raise "An existing tag (#{dupe_existing_tag}) is the same, consider using update_tag?"
       end

lib/dor/models/releaseable.rb

@@ -103,7 +103,7 @@ def combine_two_release_tag_hashes(hash_one, hash_two)
     def get_tags_for_what_value(tags, what_target)
       return_hash = {}
       tags.keys.each do |key|
-        self_tags = tags[key].select {|tag| tag['what'] == what_target.downcase}
+        self_tags = tags[key].select {|tag| tag['what'].casecmp(what_target) == 0}
         return_hash[key] = self_tags if self_tags.size > 0
       end
       return_hash
@@ -180,7 +180,7 @@ def release_tag_node_to_hash(rtag)
       attrs = rtag.attributes
       return_hash = { :to => attrs[to].value }
       attrs.tap { |a| a.delete(to) }
-      attrs[release] = rtag.text.downcase == 'true' # save release as a boolean
+      attrs[release] = rtag.text.casecmp('true') == 0 # save release as a boolean
       return_hash[:attrs] = attrs
 
       # convert all the attrs beside :to to strings, they are currently Nokogiri::XML::Attr
@@ -316,7 +316,7 @@ def form_purl_url
     def get_release_tags_from_purl_xml(doc)
       nodes = doc.xpath('//html/body/publicobject/releasedata').children
       # We only want the nodes with a name that isn't text
-      nodes.reject {|n| n.name.nil? || n.name.downcase == 'text'}.map {|n| n.attr('to')}.uniq
+      nodes.reject {|n| n.name.nil? || n.name.casecmp('text') == 0 }.map {|n| n.attr('to')}.uniq
     end
 
     # Pull all release nodes from the public xml obtained via the purl query

script/manual_integration.rb

@@ -15,7 +15,7 @@ def tar(path)
       Gem::Package::TarWriter.new(tarfile) do |tar|
         Dir[File.join(path, '**/*')].each do |file|
           mode = File.stat(file).mode
-          relative_file = file.sub /^#{Regexp.escape path}\/?/, ''
+          relative_file = file.sub(/^#{Regexp.escape path}\/?/, '')
 
           if File.directory?(file)
             tar.mkdir relative_file, mode

spec/dor/processable_spec.rb

@@ -186,7 +186,7 @@ class ProcessableWithApoItem < ActiveFedora::Base
       @item = instantiate_fixture('druid:ab123cd4567', ProcessableOnlyItem)
       solr_doc = @item.to_solr
       # the facet field should have a date in it.
-      expect(solr_doc['modified_latest_dttsi']).to match /^\d\d\d\d-\d\d-\d\dT\d\d:\d\d:\d\dZ$/
+      expect(solr_doc['modified_latest_dttsi']).to match(/^\d\d\d\d-\d\d-\d\dT\d\d:\d\d:\d\dZ$/)
     end
     it 'should create a version field for each version, including the version number, tag and description' do
       expect(Dor.logger).to receive(:warn).with(/Cannot index druid:ab123cd4567\.descMetadata.*Dor::Item#generate_dublin_core produced incorrect xml/)