Class has too many lines. [151/100] Open
class SharedPipelineGenerator
attr_reader :options, :warnings
attr_accessor :erb_context
alias :ctxt :erb_context
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks if the length a class exceeds some maximum value. Comment lines can optionally be ignored. The maximum allowed length is configurable.
Assignment Branch Condition size for set_context is too high. [83.39/15] Open
def set_context
shared_config = File.join(options.paas_templates_path, 'shared-config.yml')
private_config = File.join(options.secrets_path, 'private-config.yml')
profiles = if options.profiles_auto_sort
options.profiles.sort
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks that the ABC size of methods is not higher than the configured maximum. The ABC size is based on assignments, branches (method calls), and conditions. See http://c2.com/cgi/wiki?AbcMetric
Method has too many lines. [40/10] Open
def option_parser(options)
OptionParser.new do |opts|
opts.banner = "Incomplete/wrong parameter(s): #{opts.default_argv}.\n Usage: ./#{opts.program_name} <options>"
opts.on('-d', '--depls ROOT_DEPLOYMENT', 'Specify a root deployment name to generate template for') do |deployment_string|
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks if the length of a method exceeds some maximum value. Comment lines can optionally be ignored. The maximum allowed length is configurable.
Method has too many lines. [38/10] Open
def set_context
shared_config = File.join(options.paas_templates_path, 'shared-config.yml')
private_config = File.join(options.secrets_path, 'private-config.yml')
profiles = if options.profiles_auto_sort
options.profiles.sort
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks if the length of a method exceeds some maximum value. Comment lines can optionally be ignored. The maximum allowed length is configurable.
Assignment Branch Condition size for check_warnings is too high. [32.56/15] Open
def check_warnings
warnings << "### WARNING ### no deployment detected. Please check\n template_dir: #{options.paas_templates_path}\n secrets_dir: #{options.secrets_path}" if ctxt.multi_root_dependencies&.empty?
warnings << '### WARNING ### no ci deployment detected. Please check a valid ci-deployment-overview.yml exists' if ctxt.root_deployments.map { |name| ctxt.multi_root_ci_deployments&.empty? || ctxt.multi_root_ci_deployments[name]&.empty? }
.inject { |memo, element| memo && element }
warnings << '### WARNING ### no cf app deployment detected. Please check a valid enable-cf-app.yml exists' if ctxt.multi_root_cf_apps&.empty?
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks that the ABC size of methods is not higher than the configured maximum. The ABC size is based on assignments, branches (method calls), and conditions. See http://c2.com/cgi/wiki?AbcMetric
Assignment Branch Condition size for option_parser is too high. [21.26/15] Open
def option_parser(options)
OptionParser.new do |opts|
opts.banner = "Incomplete/wrong parameter(s): #{opts.default_argv}.\n Usage: ./#{opts.program_name} <options>"
opts.on('-d', '--depls ROOT_DEPLOYMENT', 'Specify a root deployment name to generate template for') do |deployment_string|
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks that the ABC size of methods is not higher than the configured maximum. The ABC size is based on assignments, branches (method calls), and conditions. See http://c2.com/cgi/wiki?AbcMetric
File shared_pipeline_generator.rb
has 262 lines of code (exceeds 250 allowed). Consider refactoring. Open
require 'optparse'
require_relative './config'
require_relative './extended_config'
require_relative './bosh_certificates'
- Create a ticketCreate a ticket
Cyclomatic complexity for check_warnings is too high. [7/6] Open
def check_warnings
warnings << "### WARNING ### no deployment detected. Please check\n template_dir: #{options.paas_templates_path}\n secrets_dir: #{options.secrets_path}" if ctxt.multi_root_dependencies&.empty?
warnings << '### WARNING ### no ci deployment detected. Please check a valid ci-deployment-overview.yml exists' if ctxt.root_deployments.map { |name| ctxt.multi_root_ci_deployments&.empty? || ctxt.multi_root_ci_deployments[name]&.empty? }
.inject { |memo, element| memo && element }
warnings << '### WARNING ### no cf app deployment detected. Please check a valid enable-cf-app.yml exists' if ctxt.multi_root_cf_apps&.empty?
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks that the cyclomatic complexity of methods is not higher than the configured maximum. The cyclomatic complexity is the number of linearly independent paths through a method. The algorithm counts decision points and adds one.
An if statement (or unless or ?:) increases the complexity by one. An else branch does not, since it doesn't add a decision point. The && operator (or keyword and) can be converted to a nested if statement, and ||/or is shorthand for a sequence of ifs, so they also add one. Loops can be said to have an exit condition, so they add one.
Method option_parser
has 40 lines of code (exceeds 25 allowed). Consider refactoring. Open
def option_parser(options)
OptionParser.new do |opts|
opts.banner = "Incomplete/wrong parameter(s): #{opts.default_argv}.\n Usage: ./#{opts.program_name} <options>"
opts.on('-d', '--depls ROOT_DEPLOYMENT', 'Specify a root deployment name to generate template for') do |deployment_string|
- Create a ticketCreate a ticket
Method set_context
has 38 lines of code (exceeds 25 allowed). Consider refactoring. Open
def set_context
shared_config = File.join(options.paas_templates_path, 'shared-config.yml')
private_config = File.join(options.secrets_path, 'private-config.yml')
profiles = if options.profiles_auto_sort
options.profiles.sort
- Create a ticketCreate a ticket
Block has too many lines. [38/25] Open
OptionParser.new do |opts|
opts.banner = "Incomplete/wrong parameter(s): #{opts.default_argv}.\n Usage: ./#{opts.program_name} <options>"
opts.on('-d', '--depls ROOT_DEPLOYMENT', 'Specify a root deployment name to generate template for') do |deployment_string|
options[:depls] = deployment_string
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks if the length of a block exceeds some maximum value. Comment lines can optionally be ignored. The maximum allowed length is configurable. The cop can be configured to ignore blocks passed to certain methods.
SharedPipelineGenerator::Parser#option_parser has approx 27 statements Open
def option_parser(options)
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
A method with Too Many Statements
is any method that has a large number of lines.
Too Many Statements
warns about any method that has more than 5 statements. Reek's smell detector for Too Many Statements
counts +1 for every simple statement in a method and +1 for every statement within a control structure (if
, else
, case
, when
, for
, while
, until
, begin
, rescue
) but it doesn't count the control structure itself.
So the following method would score +6 in Reek's statement-counting algorithm:
def parse(arg, argv, &error)
if !(val = arg) and (argv.empty? or /\A-/ =~ (val = argv[0]))
return nil, block, nil # +1
end
opt = (val = parse_arg(val, &error))[1] # +2
val = conv_arg(*val) # +3
if opt and !arg
argv.shift # +4
else
val[0] = nil # +5
end
val # +6
end
(You might argue that the two assigments within the first @if@ should count as statements, and that perhaps the nested assignment should count as +2.)
SharedPipelineGenerator#set_context has approx 34 statements Open
def set_context
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
A method with Too Many Statements
is any method that has a large number of lines.
Too Many Statements
warns about any method that has more than 5 statements. Reek's smell detector for Too Many Statements
counts +1 for every simple statement in a method and +1 for every statement within a control structure (if
, else
, case
, when
, for
, while
, until
, begin
, rescue
) but it doesn't count the control structure itself.
So the following method would score +6 in Reek's statement-counting algorithm:
def parse(arg, argv, &error)
if !(val = arg) and (argv.empty? or /\A-/ =~ (val = argv[0]))
return nil, block, nil # +1
end
opt = (val = parse_arg(val, &error))[1] # +2
val = conv_arg(*val) # +3
if opt and !arg
argv.shift # +4
else
val[0] = nil # +5
end
val # +6
end
(You might argue that the two assigments within the first @if@ should count as statements, and that perhaps the nested assignment should count as +2.)
SharedPipelineGenerator#set_context calls 'options.secrets_path' 6 times Open
private_config = File.join(options.secrets_path, 'private-config.yml')
profiles = if options.profiles_auto_sort
options.profiles.sort
else
options.profiles
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.
Reek implements a check for Duplicate Method Call.
Example
Here's a very much simplified and contrived example. The following method will report a warning:
def double_thing()
@other.thing + @other.thing
end
One quick approach to silence Reek would be to refactor the code thus:
def double_thing()
thing = @other.thing
thing + thing
end
A slightly different approach would be to replace all calls of double_thing
by calls to @other.double_thing
:
class Other
def double_thing()
thing + thing
end
end
The approach you take will depend on balancing other factors in your code.
SharedPipelineGenerator::PipelineTemplatesFiltering has no descriptive comment Open
class PipelineTemplatesFiltering
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Classes and modules are the units of reuse and release. It is therefore considered good practice to annotate every class and module with a brief comment outlining its responsibilities.
Example
Given
class Dummy
# Do things...
end
Reek would emit the following warning:
test.rb -- 1 warning:
[1]:Dummy has no descriptive comment (IrresponsibleModule)
Fixing this is simple - just an explaining comment:
# The Dummy class is responsible for ...
class Dummy
# Do things...
end
SharedPipelineGenerator::Parser has no descriptive comment Open
class Parser
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Classes and modules are the units of reuse and release. It is therefore considered good practice to annotate every class and module with a brief comment outlining its responsibilities.
Example
Given
class Dummy
# Do things...
end
Reek would emit the following warning:
test.rb -- 1 warning:
[1]:Dummy has no descriptive comment (IrresponsibleModule)
Fixing this is simple - just an explaining comment:
# The Dummy class is responsible for ...
class Dummy
# Do things...
end
SharedPipelineGenerator has no descriptive comment Open
class SharedPipelineGenerator
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Classes and modules are the units of reuse and release. It is therefore considered good practice to annotate every class and module with a brief comment outlining its responsibilities.
Example
Given
class Dummy
# Do things...
end
Reek would emit the following warning:
test.rb -- 1 warning:
[1]:Dummy has no descriptive comment (IrresponsibleModule)
Fixing this is simple - just an explaining comment:
# The Dummy class is responsible for ...
class Dummy
# Do things...
end
SharedPipelineGenerator::ErbSharedContext has no descriptive comment Open
ErbSharedContext = Struct.new(
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Classes and modules are the units of reuse and release. It is therefore considered good practice to annotate every class and module with a brief comment outlining its responsibilities.
Example
Given
class Dummy
# Do things...
end
Reek would emit the following warning:
test.rb -- 1 warning:
[1]:Dummy has no descriptive comment (IrresponsibleModule)
Fixing this is simple - just an explaining comment:
# The Dummy class is responsible for ...
class Dummy
# Do things...
end
Method check_warnings
has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring. Open
def check_warnings
warnings << "### WARNING ### no deployment detected. Please check\n template_dir: #{options.paas_templates_path}\n secrets_dir: #{options.secrets_path}" if ctxt.multi_root_dependencies&.empty?
warnings << '### WARNING ### no ci deployment detected. Please check a valid ci-deployment-overview.yml exists' if ctxt.root_deployments.map { |name| ctxt.multi_root_ci_deployments&.empty? || ctxt.multi_root_ci_deployments[name]&.empty? }
.inject { |memo, element| memo && element }
warnings << '### WARNING ### no cf app deployment detected. Please check a valid enable-cf-app.yml exists' if ctxt.multi_root_cf_apps&.empty?
- Read upRead up
- Create a ticketCreate a ticket
Cognitive Complexity
Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.
A method's cognitive complexity is based on a few simple rules:
- Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
- Code is considered more complex for each "break in the linear flow of the code"
- Code is considered more complex when "flow breaking structures are nested"
Further reading
SharedPipelineGenerator::PipelineTemplatesFiltering#remove_excluded_pipeline_templates doesn't depend on instance state (maybe move it to another class?) Open
def remove_excluded_pipeline_templates(excluded_pipeline, pipelines)
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
A Utility Function is any instance method that has no dependency on the state of the instance.
SharedPipelineGenerator#erb_context is a writable attribute Open
attr_accessor :erb_context
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
A class that publishes a setter for an instance variable invites client classes to become too intimate with its inner workings, and in particular with its representation of state.
The same holds to a lesser extent for getters, but Reek doesn't flag those.
Example
Given:
class Klass
attr_accessor :dummy
end
Reek would emit the following warning:
reek test.rb
test.rb -- 1 warning:
[2]:Klass declares the writable attribute dummy (Attribute)
Similar blocks of code found in 2 locations. Consider refactoring. Open
class PipelineTemplatesFiltering
attr_reader :options
def initialize(options, location = "/concourse/pipelines/template")
@required_pipeline_templates = options.input_pipelines || []
- Read upRead up
- Create a ticketCreate a ticket
Duplicated Code
Duplicated code can lead to software that is hard to understand and difficult to change. The Don't Repeat Yourself (DRY) principle states:
Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.
When you violate DRY, bugs and maintenance problems are sure to follow. Duplicated code has a tendency to both continue to replicate and also to diverge (leaving bugs as two similar implementations differ in subtle ways).
Tuning
This issue has a mass of 100.
We set useful threshold defaults for the languages we support but you may want to adjust these settings based on your project guidelines.
The threshold configuration represents the minimum mass a code block must have to be analyzed for duplication. The lower the threshold, the more fine-grained the comparison.
If the engine is too easily reporting duplication, try raising the threshold. If you suspect that the engine isn't catching enough duplication, try lowering the threshold. The best setting tends to differ from language to language.
See codeclimate-duplication
's documentation for more information about tuning the mass threshold in your .codeclimate.yml
.
Refactorings
- Extract Method
- Extract Class
- Form Template Method
- Introduce Null Object
- Pull Up Method
- Pull Up Field
- Substitute Algorithm
Further Reading
- Don't Repeat Yourself on the C2 Wiki
- Duplicated Code on SourceMaking
- Refactoring: Improving the Design of Existing Code by Martin Fowler. Duplicated Code, p76
Identical blocks of code found in 2 locations. Consider refactoring. Open
DEFAULT_OPTIONS = {
depls: 'shared',
git_submodule_path: '../paas-templates',
secrets_path: '..',
output_path: 'bootstrap-generated',
- Read upRead up
- Create a ticketCreate a ticket
Duplicated Code
Duplicated code can lead to software that is hard to understand and difficult to change. The Don't Repeat Yourself (DRY) principle states:
Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.
When you violate DRY, bugs and maintenance problems are sure to follow. Duplicated code has a tendency to both continue to replicate and also to diverge (leaving bugs as two similar implementations differ in subtle ways).
Tuning
This issue has a mass of 27.
We set useful threshold defaults for the languages we support but you may want to adjust these settings based on your project guidelines.
The threshold configuration represents the minimum mass a code block must have to be analyzed for duplication. The lower the threshold, the more fine-grained the comparison.
If the engine is too easily reporting duplication, try raising the threshold. If you suspect that the engine isn't catching enough duplication, try lowering the threshold. The best setting tends to differ from language to language.
See codeclimate-duplication
's documentation for more information about tuning the mass threshold in your .codeclimate.yml
.
Refactorings
- Extract Method
- Extract Class
- Form Template Method
- Introduce Null Object
- Pull Up Method
- Pull Up Field
- Substitute Algorithm
Further Reading
- Don't Repeat Yourself on the C2 Wiki
- Duplicated Code on SourceMaking
- Refactoring: Improving the Design of Existing Code by Martin Fowler. Duplicated Code, p76
Extra empty line detected at method body end. Open
end
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cops checks if empty lines exist around the bodies of methods.
Example:
# good
def foo
# ...
end
# bad
def bar
# ...
end
Extra blank line detected. Open
class SharedPipelineGenerator
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cops checks for two or more consecutive blank lines.
Example:
# bad - It has two empty lines.
some_method
# one empty line
# two empty lines
some_method
# good
some_method
# one empty line
some_method
Use alias ctxt erb_context
instead of alias :ctxt :erb_context
. Open
alias :ctxt :erb_context
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop enforces the use of either #alias
or #alias_method
depending on configuration.
It also flags uses of alias :symbol
rather than alias bareword
.
Example: EnforcedStyle: prefer_alias (default)
# bad
alias_method :bar, :foo
alias :bar :foo
# good
alias bar foo
Example: EnforcedStyle: preferaliasmethod
# bad
alias :bar :foo
alias bar foo
# good
alias_method :bar, :foo
Use 4 (not 119) spaces for indenting a condition in an if
statement spanning multiple lines. Open
.inject { |memo, element| memo && element }
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks the indentation of the method name part in method calls that span more than one line.
Example: EnforcedStyle: aligned
# bad
while myvariable
.b
# do something
end
# good
while myvariable
.b
# do something
end
# good
Thing.a
.b
.c
Example: EnforcedStyle: indented
# good
while myvariable
.b
# do something
end
Example: EnforcedStyle: indentedrelativeto_receiver
# good
while myvariable
.a
.b
# do something
end
# good
myvariable = Thing
.a
.b
.c
Useless assignment to variable - deployment_versions_path
. Open
deployment_versions_path = "#{options.paas_templates_path}/#{options.depls}/root-deployment.yml"
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks for every useless assignment to local variable in every
scope.
The basic idea for this cop was from the warning of ruby -cw
:
assigned but unused variable - foo
Currently this cop has advanced logic that detects unreferenced reassignments and properly handles varied cases such as branch, loop, rescue, ensure, etc.
Example:
# bad
def some_method
some_var = 1
do_something
end
Example:
# good
def some_method
some_var = 1
do_something(some_var)
end
Redundant self
detected. Open
processor = TemplateProcessor.new(root_deployment_name, options.to_h, self.ctxt.to_h)
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks for redundant uses of self
.
The usage of self
is only needed when:
Sending a message to same object with zero arguments in presence of a method name clash with an argument or a local variable.
Calling an attribute writer to prevent an local variable assignment.
Note, with using explicit self you can only send messages with public or protected scope, you cannot send private messages this way.
Note we allow uses of self
with operators because it would be awkward
otherwise.
Example:
# bad
def foo(bar)
self.baz
end
# good
def foo(bar)
self.bar # Resolves name clash with the argument.
end
def foo
bar = 1
self.bar # Resolves name clash with the local variable.
end
def foo
%w[x y z].select do |bar|
self.bar == bar # Resolves name clash with argument of the block.
end
end
Extra empty line detected at method body end. Open
end
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cops checks if empty lines exist around the bodies of methods.
Example:
# good
def foo
# ...
end
# bad
def bar
# ...
end
Interpolation in single quoted string detected. Use double quoted strings if you need interpolation. Open
puts 'ERROR: use -a option to set cf-ops-automation root dir <AUTOMATION_ROOT_DIR>/#{templates_location}'
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks for interpolation in a single quoted string.
Example:
# bad
foo = 'something with #{interpolation} inside'
Example:
# good
foo = "something with #{interpolation} inside"
Freeze mutable objects assigned to constants. Open
BOSH_CERT_LOCATIONS = {
'on-demand-depls' => 'shared/certs/internal_paas-ca/server-ca.crt',
'micro-depls' => 'shared/certs/internal_paas-ca/server-ca.crt',
'master-depls' => 'shared/certs/internal_paas-ca/server-ca.crt',
'expe-depls' => 'shared/certs/internal_paas-ca/server-ca.crt',
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks whether some constant value isn't a mutable literal (e.g. array or hash).
Example:
# bad
CONST = [1, 2, 3]
# good
CONST = [1, 2, 3].freeze