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 1f8cf4b commit 5d4ea14
Show file tree
Hide file tree
Showing 59 changed files with 146 additions and 130 deletions.
10 changes: 10 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,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
6 changes: 3 additions & 3 deletions 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 All @@ -198,12 +198,12 @@ def interface(type_string)
end

def implementation(&block)
raise ArgumentError, _('a data type can only have one implementation') if @type_builder.has_implementation?
raise ArgumentError, _('a data type can only have one implementation') if @type_builder.implementation?
@type_builder.implementation = block
end

def implementation_class(ruby_class)
raise ArgumentError, _('a data type can only have one implementation') if @type_builder.has_implementation?
raise ArgumentError, _('a data type can only have one implementation') if @type_builder.implementation?
@type_builder.implementation_class = ruby_class
end

Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/face/help.rb
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ def exclude_from_docs?(appname)
# See #14205.
#private :exclude_from_docs?

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

clazz.ancestors.include?(Puppet::Application::FaceBase)
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/feature/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ module WindowsSymlink
require 'ffi'
extend FFI::Library

def self.is_implemented
def self.is_implemented # rubocop:disable Naming/PredicateName
begin
ffi_lib :kernel32
attach_function :CreateSymbolicLinkW, [:lpwstr, :lpwstr, :dword], :boolean
Expand Down
3 changes: 2 additions & 1 deletion lib/puppet/functions/defined.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@
required_repeated_param 'Variant[String, Type[CatalogEntry], Type[Type[CatalogEntry]]]', :vals
end

def is_defined(scope, *vals)

def is_defined(scope, *vals) # rubocop:disable Naming/PredicateName
vals.any? do |val|
case val
when String
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/generate/models/type/property.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def initialize(property)

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

Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/graph/rb_tree_map.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def push(key, value)
# map.push("GA", "Georgia")
# map.has_key?("GA") #=> true
# map.has_key?("DE") #=> false
def has_key?(key)
def has_key?(key) # rubocop:disable Naming/PredicateName
!get_recursive(@root, key).nil?
end

Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/interface/option.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def required?
!!@required
end

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

Expand Down
5 changes: 3 additions & 2 deletions lib/puppet/module/plan.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,14 @@ 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}

def self.is_plan_name?(name)

def self.is_plan_name?(name) # rubocop:disable Naming/PredicateName
return true if name =~ /^[a-z][a-z0-9_]*$/
return false
end

# Determine whether a plan file has a legal name and extension
def self.is_plans_filename?(path)
def self.is_plans_filename?(path) # rubocop:disable Naming/PredicateName
name = File.basename(path, '.*')
ext = File.extname(path)
return [false, _("Plan names must start with a lowercase letter and be composed of only lowercase letters, numbers, and underscores")] unless is_plan_name?(name)
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/module_tool.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ 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
def self.is_module_root?(path)
def self.is_module_root?(path) # rubocop:disable Naming/PredicateName
path = Pathname.new(path) if path.class == String

FileTest.file?(path + 'metadata.json')
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def environment=(env)
end
end

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

Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pal/catalog_compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def catalog
# This implementation returns `true`
# @return [Boolean] true
# @api public
def has_catalog?
def has_catalog? # rubocop:disable Naming/PredicateName
true
end

Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pal/compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ def create(data_type, *arguments)
# Returns true if this is a compiler that compiles a catalog.
# This implementation returns `false`
# @return Boolan false
def has_catalog?
def has_catalog? # rubocop:disable Naming/PredicateName
false
end

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: 6 additions & 6 deletions lib/puppet/parser/ast/pops_bridge.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,18 +118,18 @@ def each
end

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

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,7 +231,7 @@ def code()
Expression.new(:value => @value)
end

def is_nop?(o)
def is_nop?(o) # rubocop:disable Naming/PredicateName
@ast_transformer.is_nop?(o)
end

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: 1 addition & 1 deletion lib/puppet/parser/templatewrapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def script_line

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

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
4 changes: 2 additions & 2 deletions lib/puppet/pops/evaluator/runtime3_support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ def get_resource_parameter_value(scope, resource, parameter_name)

# Returns true, if the given name is the name of a resource parameter.
#
def is_parameter_of_resource?(scope, resource, name)
def parameter_of_resource?(scope, resource, name)
return false unless name.is_a?(String)
resource.valid_parameter?(name)
end
Expand All @@ -446,7 +446,7 @@ def resource_to_ptype(resource)

# This is the same type of "truth" as used in the current Puppet DSL.
#
def is_true?(value, o)
def true?(value, o)
# Is the value true? This allows us to control the definition of truth
# in one place.
case value
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pops/loader/loader_paths.rb
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ def valid_path?(path)
path.start_with?(generic_path) && is_task_name?(File.basename(path, '.*')) && !FORBIDDEN_EXTENSIONS.any? { |ext| path.end_with?(ext) }
end

def is_task_name?(name)
def is_task_name?(name) # rubocop:disable Naming/PredicateName
!!(name =~ /^[a-z][a-z0-9_]*$/)
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pops/lookup/global_data_provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def unchecked_key_lookup(key, lookup_invocation, merge)

# Confine to global scope unless an environment data provider has been defined (same as for hiera_xxx functions)
adapter = lookup_invocation.lookup_adapter
hiera_invocation.set_global_only unless adapter.global_only? || adapter.has_environment_data_provider?(lookup_invocation)
hiera_invocation.set_global_only unless adapter.global_only? || adapter.environment_data_provider?(lookup_invocation)
hiera_invocation.lookup(key, lookup_invocation.module_name) { unchecked_key_lookup(key , hiera_invocation, merge) }
end
end
Expand Down
Loading

0 comments on commit 5d4ea14

Please sign in to comment.