diff --git a/lib/puppet-lint.rb b/lib/puppet-lint.rb index 7926014d..b024d08c 100644 --- a/lib/puppet-lint.rb +++ b/lib/puppet-lint.rb @@ -235,6 +235,42 @@ def print_problems report(@problems) end + # Public: Write fixes back to the file if this file type supports fixes. + # + # Returns nothing. + def write_fixes + return unless should_write_fixes? + + File.binwrite(@path, @manifest) + end + + # Internal: Determine if fixes should be written for this file. + # + # Returns true if all conditions are met for writing fixes, false otherwise. + def should_write_fixes? + # Don't write if file type doesn't support fixes + return false unless supports_fixes? + + # Don't write if there are syntax errors (can't safely fix) + return false if @problems&.any? { |r| r[:check] == :syntax } + + # Don't write if there's no manifest content + return false if @manifest.nil? || @manifest.empty? + + true + end + + # Public: Determine if this file type supports automatic fixes. + # + # Returns true if fixes are supported for this file type, false otherwise. + def supports_fixes? + return false if @path.nil? + + # Only .pp files support fixes currently + # YAML files and other types may support fixes in the future + File.extname(@path).match?(%r{\.pp$}i) + end + # Public: Define a new check. # # name - A unique name for the check as a Symbol. diff --git a/lib/puppet-lint/bin.rb b/lib/puppet-lint/bin.rb index 593d06ae..49205338 100644 --- a/lib/puppet-lint/bin.rb +++ b/lib/puppet-lint/bin.rb @@ -90,9 +90,7 @@ def run return_val = 1 if l.errors? || (l.warnings? && PuppetLint.configuration.fail_on_warnings) - next unless PuppetLint.configuration.fix && l.problems.none? { |r| r[:check] == :syntax } - - File.binwrite(f, l.manifest) + l.write_fixes if PuppetLint.configuration.fix end if PuppetLint.configuration.sarif diff --git a/spec/fixtures/test/manifests/issue_254_overwriting_yaml/class_with_dash.pp b/spec/fixtures/test/manifests/issue_254_overwriting_yaml/class_with_dash.pp new file mode 100644 index 00000000..83d5c633 --- /dev/null +++ b/spec/fixtures/test/manifests/issue_254_overwriting_yaml/class_with_dash.pp @@ -0,0 +1,3 @@ +class foo-bar { + notify { 'hello': } +} diff --git a/spec/fixtures/test/manifests/issue_254_overwriting_yaml/class_with_dash.yaml b/spec/fixtures/test/manifests/issue_254_overwriting_yaml/class_with_dash.yaml new file mode 100644 index 00000000..c2b5a50f --- /dev/null +++ b/spec/fixtures/test/manifests/issue_254_overwriting_yaml/class_with_dash.yaml @@ -0,0 +1,7 @@ +--- +classes: + foo-bar: + parameters: [] + resources: + notify: + - title: hello diff --git a/spec/unit/puppet-lint/bin_spec.rb b/spec/unit/puppet-lint/bin_spec.rb index 542ef7e6..c6f932c8 100644 --- a/spec/unit/puppet-lint/bin_spec.rb +++ b/spec/unit/puppet-lint/bin_spec.rb @@ -615,9 +615,27 @@ def initialize(args) end end + context 'when fixing a directory containing both .pp and .yaml files' do + let(:args) { ['--fix', 'spec/fixtures/test/manifests/issue_254_overwriting_yaml'] } + + its(:exitstatus) { is_expected.to eq(1) } + + it 'does not overwrite YAML files' do + yaml_file = 'spec/fixtures/test/manifests/issue_254_overwriting_yaml/class_with_dash.yaml' + original_yaml = File.read(yaml_file) + + bin # Run the command + + yaml_after = File.read(yaml_file) + expect(yaml_after).to eq(original_yaml) + expect(yaml_after).to include('classes:') + expect(yaml_after).not_to include('class foo-bar {') + end + end + context 'when overriding config file options with command line options' do context 'and config file sets "--only-checks=variable_contains_dash"' do - around(:context) do |example| + around(:each) do |example| Dir.mktmpdir do |tmpdir| Dir.chdir(tmpdir) do File.open('.puppet-lint.rc', 'wb') do |f| diff --git a/spec/unit/puppet-lint/puppet-lint_spec.rb b/spec/unit/puppet-lint/puppet-lint_spec.rb index 7cfa231e..0f17e247 100644 --- a/spec/unit/puppet-lint/puppet-lint_spec.rb +++ b/spec/unit/puppet-lint/puppet-lint_spec.rb @@ -15,4 +15,53 @@ linter.run expect(linter.manifest).to eq('') end + + describe '#supports_fixes?' do + context 'with a .pp file' do + it 'returns true' do + linter.instance_variable_set(:@path, 'test.pp') + expect(linter.supports_fixes?).to be true + end + end + + context 'with a .yaml file' do + it 'returns false' do + linter.instance_variable_set(:@path, 'test.yaml') + expect(linter.supports_fixes?).to be false + end + end + + context 'with no path set' do + it 'returns false' do + expect(linter.supports_fixes?).to be false + end + end + end + + describe '#should_write_fixes?' do + before(:each) do + linter.instance_variable_set(:@path, 'test.pp') + linter.instance_variable_set(:@manifest, 'class test { }') + end + + context 'when file supports fixes and has no syntax errors' do + it 'returns true' do + expect(linter.should_write_fixes?).to be true + end + end + + context 'when file has syntax errors' do + it 'returns false' do + linter.instance_variable_set(:@problems, [{ check: :syntax }]) + expect(linter.should_write_fixes?).to be false + end + end + + context 'when file type does not support fixes' do + it 'returns false' do + linter.instance_variable_set(:@path, 'test.yaml') + expect(linter.should_write_fixes?).to be false + end + end + end end