Merge pull request #23 from highb/restructure_supported_decorators

Restructure supported decorators, add RuboCop and basic fixes

.rubocop.yml

@@ -0,0 +1 @@
+inherit_from: .rubocop_todo.yml

.rubocop_todo.yml

@@ -0,0 +1,69 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2017-10-13 16:58:50 -0700 using RuboCop version 0.49.1.
+# 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: 4
+Metrics/AbcSize:
+  Max: 31
+
+# Offense count: 5
+# Configuration parameters: CountComments, ExcludedMethods.
+Metrics/BlockLength:
+  Max: 68
+
+# Offense count: 1
+# Configuration parameters: CountComments.
+Metrics/ClassLength:
+  Max: 106
+
+# Offense count: 3
+Metrics/CyclomaticComplexity:
+  Max: 8
+
+# Offense count: 86
+# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
+# URISchemes: http, https
+Metrics/LineLength:
+  Max: 174
+
+# Offense count: 6
+# Configuration parameters: CountComments.
+Metrics/MethodLength:
+  Max: 26
+
+# Offense count: 2
+Metrics/PerceivedComplexity:
+  Max: 8
+
+# Offense count: 2
+Style/Documentation:
+  Exclude:
+    - 'spec/**/*'
+    - 'test/**/*'
+    - 'lib/rubocop/cop/i18n/gettext.rb'
+    - 'lib/rubocop/cop/i18n/gettext/decorate_function_message.rb'
+
+# Offense count: 1
+# Configuration parameters: ExpectMatchingDefinition, Regex, IgnoreExecutableScripts, AllowedAcronyms.
+# AllowedAcronyms: CLI, DSL, ACL, API, ASCII, CPU, CSS, DNS, EOF, GUID, HTML, HTTP, HTTPS, ID, IP, JSON, LHS, QPS, RAM, RHS, RPC, SLA, SMTP, SQL, SSH, TCP, TLS, TTL, UDP, UI, UID, UUID, URI, URL, UTF8, VM, XML, XMPP, XSRF, XSS
+Style/FileName:
+  Exclude:
+    - 'lib/rubocop-i18n.rb'
+
+# Offense count: 7
+# Configuration parameters: SupportedStyles.
+# SupportedStyles: annotated, template
+Style/FormatStringToken:
+  EnforcedStyle: template
+
+# Offense count: 4
+# Configuration parameters: MinBodyLength.
+Style/GuardClause:
+  Exclude:
+    - 'lib/rubocop/cop/i18n/gettext/decorate_function_message.rb'
+    - 'lib/rubocop/cop/i18n/gettext/decorate_string.rb'
+    - 'lib/rubocop/cop/i18n/gettext/decorate_string_formatting_using_interpolation.rb'
+    - 'lib/rubocop/cop/i18n/gettext/decorate_string_formatting_using_percent.rb'

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## master (unreleased)
 
+ * Code restructure (no API changes)
+ * RuboCop lint fixes
+
 ### 1.1.0
 
  * Added support for DecorateStringFormattingUsingPercent

Rakefile

@@ -1,6 +1,12 @@
-require "bundler/gem_tasks"
-require "rspec/core/rake_task"
+require 'bundler/gem_tasks'
+require 'rspec/core/rake_task'
 
 RSpec::Core::RakeTask.new(:spec)
 
-task :default => :spec
+task default: :test
+
+task test: %i[rubocop spec]
+
+task :rubocop do
+  sh 'rubocop -P'
+end

bin/console

@@ -1,7 +1,7 @@
 #!/usr/bin/env ruby
 
-require "bundler/setup"
-require "rubocop/i18n"
+require 'bundler/setup'
+require 'rubocop/i18n'
 
 # You can add fixtures and/or initialization code here to make experimenting
 # with your gem easier. You can also use a different console, if you like.
@@ -10,5 +10,5 @@ require "rubocop/i18n"
 # require "pry"
 # Pry.start
 
-require "irb"
+require 'irb'
 IRB.start(__FILE__)

lib/rubocop/cop/i18n/gettext.rb

@@ -4,6 +4,41 @@ module RuboCop
   module Cop
     module I18n
       module GetText
+        def self.supported_methods
+          %w[raise fail]
+        end
+
+        # Supports decorators from
+        # * mutoh/gettext https://github.com/mutoh/gettext/blob/master/lib/gettext.rb
+        # * grosser/fast_gettext https://github.com/grosser/fast_gettext/blob/master/lib/fast_gettext/translation.rb
+        def self.supported_decorators
+          %w[
+            _
+            n_
+            np_
+            ns_
+            N_
+            Nn_
+            D_
+            Dn_
+            Ds_
+            Dns_
+            d_
+            dn_
+            ds_
+            dns_
+            p_
+            s_
+          ]
+        end
+
+        def self.supported_method?(method_name)
+          supported_methods.include?(method_name)
+        end
+
+        def self.supported_decorator?(decorator_name)
+          supported_decorators.include?(decorator_name)
+        end
       end
     end
   end

lib/rubocop/cop/i18n/gettext/decorate_function_message.rb

@@ -3,35 +3,28 @@ module Cop
     module I18n
       module GetText
         class DecorateFunctionMessage < Cop
-          SUPPORTED_METHODS = ['raise', 'fail']
-          SUPPORTED_DECORATORS = ['_', 'n_', 'N_']
-
           def on_send(node)
             method_name = node.loc.selector.source
-            return if !supported_method_name?(method_name)
+            return unless GetText.supported_method?(method_name)
             _, method_name, *arg_nodes = *node
             if !arg_nodes.empty? && !already_decorated?(node) && (contains_string?(arg_nodes) || string_constant?(arg_nodes))
-              if string_constant?(arg_nodes)
-                message_section = arg_nodes[1]
-              else
-                message_section = arg_nodes[0]
-              end
+              message_section = if string_constant?(arg_nodes)
+                                  arg_nodes[1]
+                                else
+                                  arg_nodes[0]
+                                end
 
               detect_and_report(node, message_section, method_name)
             end
           end
 
           private
 
-          def supported_method_name?(method_name)
-            SUPPORTED_METHODS.include?(method_name)
-          end
-
           def already_decorated?(node, parent = nil)
             parent ||= node
 
             if node.respond_to?(:loc) && node.loc.respond_to?(:selector)
-              return true if SUPPORTED_DECORATORS.include?(node.loc.selector.source)
+              return true if GetText.supported_decorator?(node.loc.selector.source)
             end
 
             return false unless node.respond_to?(:children)
@@ -44,10 +37,10 @@ def string_constant?(nodes)
           end
 
           def contains_string?(nodes)
-            nodes[0].inspect.include?(":str") || nodes[0].inspect.include?(":dstr")
+            nodes[0].inspect.include?(':str') || nodes[0].inspect.include?(':dstr')
           end
 
-          def detect_and_report(node, message_section, method_name)
+          def detect_and_report(_node, message_section, method_name)
             errors = how_bad_is_it(message_section)
             return if errors.empty?
             error_message = "'#{method_name}' function, "
@@ -68,7 +61,7 @@ def how_bad_is_it(message_section)
             errors.push :multiline if message_section.multiline?
             errors.push :concatenation if concatenation_offense?(message_section)
             errors.push :interpolation if interpolation_offense?(message_section)
-            errors.push :no_decoration if !already_decorated?(message_section)
+            errors.push :no_decoration unless already_decorated?(message_section)
 
             # only display no_decoration, if that is the only problem.
             if errors.size > 1 && errors.include?(:no_decoration)
@@ -92,7 +85,7 @@ def concatenation_offense?(node, parent = nil)
           def interpolation_offense?(node, parent = nil)
             parent ||= node
 
-            return true if node.class == RuboCop::AST::Node  && node.dstr_type?
+            return true if node.class == RuboCop::AST::Node && node.dstr_type?
 
             return false unless node.respond_to?(:children)
 
@@ -103,52 +96,51 @@ def autocorrect(node)
             if node.str_type?
               single_string_correct(node)
             elsif interpolation_offense?(node)
-#              interpolation_correct(node)
+              #              interpolation_correct(node)
             end
           end
 
           def single_string_correct(node)
-            ->(corrector) {
-              corrector.insert_before(node.source_range , "_(")
-            corrector.insert_after(node.source_range , ")") }
+            lambda { |corrector|
+              corrector.insert_before(node.source_range, '_(')
+              corrector.insert_after(node.source_range, ')')
+            }
           end
 
           def interpolation_correct(node)
-            interpolated_values_string = ""
+            interpolated_values_string = ''
             count = 0
-            ->(corrector) {
+            lambda { |corrector|
               node.children.each do |child|
                 # dstrs are split into "str" segments and other segments.
                 # The "other" segments are the interpolated values.
-                if child.type == :begin
-                  value = child.children[0]
-                  hash_key = "value"
-                  if value.type == :lvar
-                    # Use the variable's name as the format key
-                    hash_key = value.loc.name.source
-                  else
-                    # These are placeholders that will manually need to be given
-                    # a descriptive name
-                    hash_key << "#{count}"
-                    count += 1
-                  end
-                  if interpolated_values_string.empty?
-                    interpolated_values_string << "{ "
-                  end
-                  interpolated_values_string << "#{hash_key}: #{value.loc.expression.source}, "
-
-                  # Replace interpolation with format string
-                  corrector.replace(child.loc.expression, "%{#{hash_key}}")
+                next unless child.type == :begin
+                value = child.children[0]
+                hash_key = 'value'
+                if value.type == :lvar
+                  # Use the variable's name as the format key
+                  hash_key = value.loc.name.source
+                else
+                  # These are placeholders that will manually need to be given
+                  # a descriptive name
+                  hash_key << count.to_s
+                  count += 1
+                end
+                if interpolated_values_string.empty?
+                  interpolated_values_string << '{ '
                 end
+                interpolated_values_string << "#{hash_key}: #{value.loc.expression.source}, "
+
+                # Replace interpolation with format string
+                corrector.replace(child.loc.expression, "%{#{hash_key}}")
               end
-              if !interpolated_values_string.empty?
-                interpolated_values_string << "}"
+              unless interpolated_values_string.empty?
+                interpolated_values_string << '}'
               end
               corrector.insert_before(node.source_range, '_(')
               corrector.insert_after(node.source_range, ") % #{interpolated_values_string}")
             }
           end
-
         end
       end
     end

lib/rubocop/cop/i18n/gettext/decorate_string.rb

@@ -21,9 +21,9 @@ module GetText
         class DecorateString < Cop
           def on_str(node)
             str = node.children[0]
-            #ignore strings with no whitespace - are typically keywords or interpolation statements and cover the above commented-out statements
+            # ignore strings with no whitespace - are typically keywords or interpolation statements and cover the above commented-out statements
             if str !~ /^\S*$/
-              add_offense(node, :expression, "decorator is missing around sentence") if node.loc.respond_to?(:begin)
+              add_offense(node, :expression, 'decorator is missing around sentence') if node.loc.respond_to?(:begin)
             end
           end
 
@@ -32,7 +32,6 @@ def on_str(node)
           def message(node)
             node.receiver ? MSG_DEFAULT : MSG_SELF
           end
-
         end
       end
     end

lib/rubocop/cop/i18n/gettext/decorate_string_formatting_using_interpolation.rb

@@ -3,8 +3,9 @@ module Cop
     module I18n
       module GetText
         # When using an decorated string to support I18N, any strings inside the decoration should not contain
-        # the '#{}' interpolation string as this makes it hard to translate the strings. This cop checks the
-        # decorators listed in SUPPORTED_DECORATORS
+        # the '#{}' interpolation string as this makes it hard to translate the strings.
+        #
+        # Check GetText.supported_decorators for a list of decorators that can be used.
         #
         # @example
         #
@@ -20,12 +21,9 @@ module GetText
         #   _("result is %{detail}" % {detail: message})
         #
         class DecorateStringFormattingUsingInterpolation < Cop
-
-          SUPPORTED_DECORATORS = ['_', 'n_', 'N_']
-
           def on_send(node)
             decorator_name = node.loc.selector.source
-            return if !supported_decorator_name?(decorator_name)
+            return unless GetText.supported_decorator?(decorator_name)
             _, method_name, *arg_nodes = *node
             if !arg_nodes.empty? && contains_string_formatting_with_interpolation?(arg_nodes)
               message_section = arg_nodes[0]
@@ -35,10 +33,6 @@ def on_send(node)
 
           private
 
-          def supported_decorator_name?(decorator_name)
-            SUPPORTED_DECORATORS.include?(decorator_name)
-          end
-
           def string_contains_interpolation_format?(str)
             str.match(/\#{[^}]+}/)
           end
@@ -49,17 +43,16 @@ def contains_string_formatting_with_interpolation?(node)
             end
 
             if node.respond_to?(:type)
-              if node.type == :str or node.type == :dstr
+              if node.type == :str || node.type == :dstr
                 return string_contains_interpolation_format?(node.source)
               end
             end
 
             if node.respond_to?(:children)
               return node.children.any? { |child| contains_string_formatting_with_interpolation?(child) }
             end
-            return false
+            false
           end
-
         end
       end
     end