Skip to content

Commit

Permalink
Naming/PredicateName
Browse files Browse the repository at this point in the history
This commit addresses the Rubocop Naming/PredicateName cop by either
exempting public methods with predicate names or renaming private
methods.
  • Loading branch information
mhashizume committed Dec 12, 2023
1 parent 8854123 commit 8ef4974
Show file tree
Hide file tree
Showing 59 changed files with 173 additions and 91 deletions.
10 changes: 10 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,16 @@ Naming/MethodName:
Naming/MethodParameterName:
Enabled: false

# Exclude public methods to avoid breaking changes
Naming/PredicateName:
Exclude:
- 'lib/puppet/module.rb'
- 'lib/puppet/module/task.rb'
- 'lib/puppet/parser/scope.rb'
- 'lib/puppet/pops/types/type_calculator.rb'
- 'lib/puppet/provider.rb'
- 'lib/puppet/util/rdoc/code_objects.rb'

Naming/RescuedExceptionsVariableName:
Enabled: false

Expand Down
8 changes: 0 additions & 8 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -643,14 +643,6 @@ Lint/ToJSON:
Lint/UnusedMethodArgument:
Enabled: false

# Configuration parameters: NamePrefix, ForbiddenPrefixes, AllowedMethods, MethodDefinitionMacros.
# NamePrefix: is_, has_, have_
# ForbiddenPrefixes: is_, has_, have_
# AllowedMethods: is_a?
# MethodDefinitionMacros: define_method, define_singleton_method
Naming/PredicateName:
Enabled: false

# Configuration parameters: EnforcedStyle, AllowModifiersOnSymbols.
# SupportedStyles: inline, group
Style/AccessModifierDeclarations:
Expand Down
2 changes: 1 addition & 1 deletion lib/hiera/puppet_function.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def lookup(scope, key, default, has_default, override, &default_block)
lookup_invocation = Puppet::Pops::Lookup::Invocation.new(scope, {}, {})
adapter = lookup_invocation.lookup_adapter
lookup_invocation.set_hiera_xxx_call
lookup_invocation.set_global_only unless adapter.global_only? || adapter.has_environment_data_provider?(lookup_invocation)
lookup_invocation.set_global_only unless adapter.global_only? || adapter.environment_data_provider?(lookup_invocation)
lookup_invocation.set_hiera_v3_location_overrides(override) unless override.nil? || override.is_a?(Array) && override.empty?
Puppet::Pops::Lookup.lookup(key, nil, default, has_default, merge_type, lookup_invocation, &default_block)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/datatypes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ def self._pcore_type
created_type
end

def has_implementation?
def implementation?
!(@implementation_class.nil? && @implementation.nil?)
end
end
Expand Down
2 changes: 2 additions & 0 deletions lib/puppet/face/help.rb
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ def exclude_from_docs?(appname)
# See #14205.
#private :exclude_from_docs?

# rubocop:disable Naming/PredicateName
def is_face_app?(appname)
clazz = Puppet::Application.find(appname)

Expand All @@ -239,5 +240,6 @@ def is_face_app?(appname)
# that you can't use the 'private' keyword inside of a Face definition.
# See #14205.
#private :is_face_app?
# rubocop:enable Naming/PredicateName

end
2 changes: 2 additions & 0 deletions lib/puppet/feature/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ module WindowsSymlink
require 'ffi'
extend FFI::Library

# rubocop:disable Naming/PredicateName
def self.is_implemented
begin
ffi_lib :kernel32
Expand All @@ -68,6 +69,7 @@ def self.is_implemented
false
end
end
# rubocop:enable Naming/PredicateName
end

WindowsSymlink.is_implemented
Expand Down
2 changes: 2 additions & 0 deletions lib/puppet/functions/defined.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
required_repeated_param 'Variant[String, Type[CatalogEntry], Type[Type[CatalogEntry]]]', :vals
end

# rubocop:disable Naming/PredicateName
def is_defined(scope, *vals)
vals.any? do |val|
case val
Expand Down Expand Up @@ -157,4 +158,5 @@ def is_defined(scope, *vals)
end
end
end
# rubocop:enable Naming/PredicateName
end
2 changes: 2 additions & 0 deletions lib/puppet/generate/models/type/property.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ def initialize(property)

# Determines if this property is a namevar.
# @return [Boolean] Returns true if the property is a namevar or false if not.
# rubocop:disable Naming/PredicateName
def is_namevar?
@is_namevar
end
# rubocop:enable Naming/PredicateName

# Gets the Puppet type for a property.
# @param property [Puppet::Property] The Puppet property to get the Puppet type for.
Expand Down
2 changes: 2 additions & 0 deletions lib/puppet/graph/rb_tree_map.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,11 @@ def push(key, value)
# map.push("GA", "Georgia")
# map.has_key?("GA") #=> true
# map.has_key?("DE") #=> false
# rubocop:disable Naming/PredicateName
def has_key?(key)
!get_recursive(@root, key).nil?
end
# rubocop:enable Naming/PredicateName

# Return the item associated with the key, or nil if none found.
#
Expand Down
2 changes: 2 additions & 0 deletions lib/puppet/interface/option.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,11 @@ def required?
!!@required
end

# rubocop:disable Naming/PredicateName
def has_default?
!!@default
end
# rubocop:enable Naming/PredicateName

def default=(proc)
if required
Expand Down
2 changes: 2 additions & 0 deletions lib/puppet/module/plan.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def initialize(plan_name, module_name)
callable data default enum float hash integer numeric optional pattern
resource runtime scalar string struct tuple type undef variant}

# rubocop:disable Naming/PredicateName
def self.is_plan_name?(name)
return true if name =~ /^[a-z][a-z0-9_]*$/
return false
Expand All @@ -73,6 +74,7 @@ def self.is_plans_filename?(path)
end
return [true]
end
# rubocop:enable Naming/PredicateName

# Executables list should contain the full path of all possible implementation files
def self.find_implementations(name, plan_files)
Expand Down
2 changes: 2 additions & 0 deletions lib/puppet/module_tool.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,13 @@ def self.find_module_root(path)
#
# @param path [Pathname, String] path to analyse
# @return [Boolean] true if the path is a module root, false otherwise
# rubocop:disable Naming/PredicateName
def self.is_module_root?(path)
path = Pathname.new(path) if path.class == String

FileTest.file?(path + 'metadata.json')
end
# rubocop:enable Naming/PredicateName

# Builds a formatted tree from a list of node hashes containing +:text+
# and +:dependencies+ keys.
Expand Down
2 changes: 2 additions & 0 deletions lib/puppet/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,11 @@ def environment=(env)
end
end

# rubocop:disable Naming/PredicateName
def has_environment_instance?
!@environment.nil?
end
# rubocop:enable Naming/PredicateName

def initialize(name, options = {})
raise ArgumentError, _("Node names cannot be nil") unless name
Expand Down
2 changes: 2 additions & 0 deletions lib/puppet/pal/catalog_compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ def catalog
# This implementation returns `true`
# @return [Boolean] true
# @api public
# rubocop:disable Naming/PredicateName
def has_catalog?
true
end
# rubocop:enable Naming/PredicateName

# Calls a block of code and yields a configured `JsonCatalogEncoder` to the block.
# @example Get resulting catalog as pretty printed Json
Expand Down
2 changes: 2 additions & 0 deletions lib/puppet/pal/compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,11 @@ def create(data_type, *arguments)
# Returns true if this is a compiler that compiles a catalog.
# This implementation returns `false`
# @return Boolan false
# rubocop:disable Naming/PredicateName
def has_catalog?
false
end
# rubocop:enable Naming/PredicateName

protected

Expand Down
2 changes: 2 additions & 0 deletions lib/puppet/parameter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,15 @@ def defaultto(value = nil, &block)
end
end

# rubocop:disable Naming/PredicateName
def sensitive(value = nil, &block)
if block
define_method(:is_sensitive, &block)
else
define_method(:is_sensitive) do value end
end
end
# rubocop:enable Naming/PredicateName

# Produces a documentation string.
# If an enumeration of _valid values_ has been defined, it is appended to the documentation
Expand Down
12 changes: 7 additions & 5 deletions lib/puppet/parser/ast/pops_bridge.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,18 +118,20 @@ def each
end

# Returns true if this Program only contains definitions
# rubocop:disable Naming/PredicateName
def is_definitions_only?
is_definition?(program_model)
end
# rubocop:enable Naming/PredicateName

private

def is_definition?(o)
def definition?(o)
case o
when Puppet::Pops::Model::Program
is_definition?(o.body)
definition?(o.body)
when Puppet::Pops::Model::BlockExpression
o.statements.all {|s| is_definition?(s) }
o.statements.all {|s| definition?(s) }
when Puppet::Pops::Model::Definition
true
else
Expand Down Expand Up @@ -231,8 +233,8 @@ def code()
Expression.new(:value => @value)
end

def is_nop?(o)
@ast_transformer.is_nop?(o)
def nop?(o)
@ast_transformer.nop?(o)
end

def absolute_reference(ref)
Expand Down
12 changes: 6 additions & 6 deletions lib/puppet/parser/scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ def inherited_scope
#
# @return [Puppet::Parser::Scope] The scope or nil if there is no enclosing scope
def enclosing_scope
if has_enclosing_scope?
if enclosing_scope?
if parent.is_topscope? || parent.is_nodescope?
parent
else
Expand Down Expand Up @@ -614,7 +614,7 @@ def lookup_qualified_variable(fqn, options)
begin
qs = qualified_scope(class_name)
unless qs.nil?
return qs.get_local_variable(leaf_name) if qs.has_local_variable?(leaf_name)
return qs.get_local_variable(leaf_name) if qs.local_variable?(leaf_name)
iscope = qs.inherited_scope
return lookup_qualified_variable("#{iscope.source.name}::#{leaf_name}", options) unless iscope.nil?
end
Expand All @@ -628,7 +628,7 @@ def lookup_qualified_variable(fqn, options)
end

# @api private
def has_local_variable?(name)
def local_variable?(name)
@ephemeral.last.include?(name)
end

Expand All @@ -651,10 +651,10 @@ def handle_not_found(class_name, variable_name, position, reason = nil)
variable_not_found("#{class_name}::#{variable_name}", reason)
end

def has_enclosing_scope?
def enclosing_scope?
! parent.nil?
end
private :has_enclosing_scope?
private :enclosing_scope?

def qualified_scope(classname)
klass = find_hostclass(classname)
Expand All @@ -671,7 +671,7 @@ def qualified_scope(classname)
# Optionally include the variables that are explicitly set to `undef`.
#
def to_hash(recursive = true, include_undef = false)
if recursive and has_enclosing_scope?
if recursive and enclosing_scope?
target = enclosing_scope.to_hash(recursive)
if !(inherited = inherited_scope).nil?
target.merge!(inherited.to_hash(recursive))
Expand Down
2 changes: 2 additions & 0 deletions lib/puppet/parser/templatewrapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ def script_line

# Should return true if a variable is defined, false if it is not
# @api public
# rubocop:disable Naming/PredicateName
def has_variable?(name)
scope.include?(name.to_s)
end
# rubocop:enable Naming/PredicateName

# @return [Array<String>] The list of defined classes
# @api public
Expand Down
4 changes: 2 additions & 2 deletions lib/puppet/pops/evaluator/access_operator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ def access_PResourceType(o, scope, keys)
end

result = keys.map do |k|
unless is_parameter_of_resource?(scope, resource, k)
unless parameter_of_resource?(scope, resource, k)
fail(Issues::UNKNOWN_RESOURCE_PARAMETER, @semantic,
{:type_name => o.type_name, :title => o.title, :param_name=>k})
end
Expand Down Expand Up @@ -700,7 +700,7 @@ def access_PClassType(o, scope, keys)
resource = find_resource(scope, 'class', o.class_name)
if resource
result = keys.map do |k|
if is_parameter_of_resource?(scope, resource, k)
if parameter_of_resource?(scope, resource, k)
get_resource_parameter_value(scope, resource, k)
else
fail(Issues::UNKNOWN_RESOURCE_PARAMETER, @semantic,
Expand Down
13 changes: 7 additions & 6 deletions lib/puppet/pops/evaluator/evaluator_impl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ def eval_QualifiedReference(o, scope)
end

def eval_NotExpression(o, scope)
! is_true?(evaluate(o.expr, scope), o.expr)
! true?(evaluate(o.expr, scope), o.expr)
end

def eval_UnaryMinusExpression(o, scope)
Expand Down Expand Up @@ -642,15 +642,15 @@ def eval_InExpression o, scope
# b is only evaluated if a is true
#
def eval_AndExpression o, scope
is_true?(evaluate(o.left_expr, scope), o.left_expr) ? is_true?(evaluate(o.right_expr, scope), o.right_expr) : false
true?(evaluate(o.left_expr, scope), o.left_expr) ? true?(evaluate(o.right_expr, scope), o.right_expr) : false
end

# @example
# a or b
# b is only evaluated if a is false
#
def eval_OrExpression o, scope
is_true?(evaluate(o.left_expr, scope), o.left_expr) ? true : is_true?(evaluate(o.right_expr, scope), o.right_expr)
true?(evaluate(o.left_expr, scope), o.left_expr) ? true : true?(evaluate(o.right_expr, scope), o.right_expr)
end

# Evaluates each entry of the literal list and creates a new Array
Expand Down Expand Up @@ -1050,7 +1050,7 @@ def eval_HeredocExpression o, scope
# Evaluates Puppet DSL `if`
def eval_IfExpression o, scope
scope.with_guarded_scope do
if is_true?(evaluate(o.test, scope), o.test)
if true?(evaluate(o.test, scope), o.test)
evaluate(o.then_expr, scope)
else
evaluate(o.else_expr, scope)
Expand All @@ -1061,7 +1061,7 @@ def eval_IfExpression o, scope
# Evaluates Puppet DSL `unless`
def eval_UnlessExpression o, scope
scope.with_guarded_scope do
unless is_true?(evaluate(o.test, scope), o.test)
unless true?(evaluate(o.test, scope), o.test)
evaluate(o.then_expr, scope)
else
evaluate(o.else_expr, scope)
Expand Down Expand Up @@ -1280,10 +1280,11 @@ def delete(x, y)
#
# This is the type of matching performed in a case option, using == for every type
# of value except regular expression where a match is performed.
#
# rubocop:disable Naming/PredicateName
def is_match?(left, right, o, option_expr, scope)
@@compare_operator.match(left, right, scope)
end
# rubocop:enable Naming/PredicateName

# Maps the expression in the given array to their product except for UnfoldExpressions which are first unfolded.
# The result is added to the given result Array.
Expand Down
Loading

0 comments on commit 8ef4974

Please sign in to comment.