Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions lib/puppet/pops/validation/checker4_0.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,20 @@ def ends_with_idem(o)
end
end

# Returns an idempotent expression whose value is discarded when +o+ is used as a statement.
# A case expression can have side effects before its branch result, so inspect each branch tail
# even when the case expression as a whole is not idempotent.
def discarded_value_expression(o)
return o if idem(o)
return nil unless o.is_a?(Model::CaseExpression)

o.options.each do |option|
violator = ends_with_idem(option.then_expr)
return violator if violator
end
nil
end

#---ASSIGNMENT CHECKS

def assign_VariableExpression(o, via_index)
Expand Down Expand Up @@ -269,8 +283,9 @@ def check_BlockExpression(o)
acceptor.accept(Issues::RESOURCE_WITHOUT_TITLE, o, :name => o.statements[0].value)
else
o.statements[0..-2].each do |statement|
if idem(statement)
acceptor.accept(Issues::IDEM_EXPRESSION_NOT_LAST, statement)
violator = discarded_value_expression(statement)
if violator
acceptor.accept(Issues::IDEM_EXPRESSION_NOT_LAST, violator)
break # only flag the first
end
end
Expand Down
32 changes: 32 additions & 0 deletions spec/unit/pops/validator/validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,38 @@ def with_environment(environment, env_params = {})
end

context 'for non productive expressions' do
it 'flags an ineffective conditional at the end of a discarded case branch' do
source = <<~'PUPPET'
case 'Ubuntu' {
'Ubuntu': {
$packages = if true { ['base'] } else { [] }
if true {
$packages << 'extra'
}
}
}
$x = 1
PUPPET

expect(validate(parse(source))).to have_issue(Puppet::Pops::Issues::IDEM_EXPRESSION_NOT_LAST)
end

it 'allows a final value when the case result is used' do
source = <<~PUPPET
class example {
$packages = case $os_name {
'Ubuntu': {
$base = ['base']
if $virtual { $base + ['vm'] } else { $base }
}
default: { [] }
}
notice($packages)
}
PUPPET

expect(validate(parse(source))).not_to have_issue(Puppet::Pops::Issues::IDEM_EXPRESSION_NOT_LAST)
end
[ '1',
'3.14',
"'a'",
Expand Down