Fix bug with codeowners file validation when existing file has new li…

…ne (#50)

* Fix a bug that results in false positive failure for codeowners validation

* Bump version

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    code_ownership (1.32.4)
+    code_ownership (1.32.5)
       code_teams (~> 1.0)
       packs
       sorbet-runtime

code_ownership.gemspec

@@ -1,6 +1,6 @@
 Gem::Specification.new do |spec|
   spec.name          = "code_ownership"
-  spec.version       = '1.32.4'
+  spec.version       = '1.32.5'
   spec.authors       = ['Gusto Engineers']
   spec.email         = ['[email protected]']
   spec.summary       = 'A gem to help engineering teams declare ownership of code'

lib/code_ownership/private/codeowners_file.rb

@@ -13,7 +13,16 @@ class CodeownersFile
 
       sig { returns(T::Array[String]) }
       def self.actual_contents_lines
-        (path.exist? ? path.read : "").split("\n")
+        if !path.exist?
+          [""]
+        else
+          content = path.read
+          lines = path.read.split("\n")
+          if content.end_with?("\n")
+            lines << ""
+          end
+          lines
+        end
       end
 
       sig { returns(T::Array[T.nilable(String)]) }
@@ -64,9 +73,9 @@ def self.expected_contents_lines
 
         [
           *header.split("\n"),
-          nil, # For line between header and codeowners_file_lines
+          "", # For line between header and codeowners_file_lines
           *codeowners_file_lines,
-          nil, # For end-of-file newline
+          "", # For end-of-file newline
         ]
       end
 

lib/code_ownership/private/validations/github_codeowners_up_to_date.rb

@@ -14,8 +14,10 @@ def validation_errors(files:, autocorrect: true, stage_changes: true)
 
           actual_content_lines = CodeownersFile.actual_contents_lines
           expected_content_lines = CodeownersFile.expected_contents_lines
-          codeowners_up_to_date = actual_content_lines == expected_content_lines
+          missing_lines = expected_content_lines - actual_content_lines
+          extra_lines = actual_content_lines - expected_content_lines
 
+          codeowners_up_to_date = !missing_lines.any? && !extra_lines.any?
           errors = T.let([], T::Array[String])
 
           if !codeowners_up_to_date
@@ -26,8 +28,6 @@ def validation_errors(files:, autocorrect: true, stage_changes: true)
               end
             else
               # If there is no current file or its empty, display a shorter message.
-              missing_lines = expected_content_lines - actual_content_lines
-              extra_lines = actual_content_lines - expected_content_lines
 
               missing_lines_text = if missing_lines.any?
                 <<~COMMENT
@@ -53,7 +53,7 @@ def validation_errors(files:, autocorrect: true, stage_changes: true)
                 ""
               end
 
-              if actual_content_lines == []
+              if actual_content_lines == [""]
                 errors << <<~CODEOWNERS_ERROR
                   CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
                 CODEOWNERS_ERROR

spec/lib/code_ownership/private/validations/github_codeowners_up_to_date_spec.rb

@@ -314,11 +314,9 @@ module CodeOwnership
                 CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
 
                 CODEOWNERS should contain the following lines, but does not:
-                - ""
                 - "/packs/my_pack/owned_file.rb @MyOrg/bar-team"
                 - "# Owner metadata key in package.yml"
                 - "/packs/my_other_package/**/** @MyOrg/bar-team"
-                - ""
 
                 CODEOWNERS should not contain the following lines, but it does:
                 - "/frontend/some/extra/line/that/should/not/exist @MyOrg/bar-team"
@@ -420,17 +418,53 @@ module CodeOwnership
                 CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
 
                 CODEOWNERS should contain the following lines, but does not:
-                - ""
                 - "/packs/my_pack/owned_file.rb @MyOrg/bar-team"
                 - "# Owner metadata key in package.yml"
                 - "/packs/my_other_package/**/** @MyOrg/bar-team"
-                - ""
 
                 See https://github.com/rubyatscale/code_ownership#README.md for more details
               EXPECTED
             end
           end
         end
+
+        context 'in an application with a CODEOWNERS file with no issue' do
+          before { create_non_empty_application }
+
+          it 'prints out the diff' do
+            FileUtils.mkdir('.github')
+            codeowners_path.write <<~CODEOWNERS
+              # STOP! - DO NOT EDIT THIS FILE MANUALLY
+              # This file was automatically generated by "bin/codeownership validate".
+              #
+              # CODEOWNERS is used for GitHub to suggest code/file owners to various GitHub
+              # teams. This is useful when developers create Pull Requests since the
+              # code/file owner is notified. Reference GitHub docs for more details:
+              # https://help.github.com/en/articles/about-code-owners
+
+
+              # Annotations at the top of file
+              /frontend/javascripts/packages/my_package/owned_file.jsx @MyOrg/bar-team
+              /packs/my_pack/owned_file.rb @MyOrg/bar-team
+
+              # Owner metadata key in package.yml
+              /packs/my_other_package/**/** @MyOrg/bar-team
+
+              # Team-specific owned globs
+              /app/services/bar_stuff/** @MyOrg/bar-team
+              /frontend/javascripts/bar_stuff/** @MyOrg/bar-team
+
+              # Owner metadata key in package.json
+              /frontend/javascripts/packages/my_other_package/**/** @MyOrg/bar-team
+
+              # Team YML ownership
+              /config/teams/bar.yml @MyOrg/bar-team
+            CODEOWNERS
+
+            expect_any_instance_of(codeowners_validation).to_not receive(:`) # rubocop:disable RSpec/AnyInstance
+            expect { CodeOwnership.validate!(autocorrect: false) }.to_not raise_error
+          end
+        end
       end
 
       context 'code_ownership.yml has skip_codeowners_validation set' do