Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

rubocop/lines: prefer assert_path_exists and refute_path_exists #19313

Merged
merged 2 commits into from
Feb 18, 2025

Conversation

nandahkrishna
Copy link
Member

@nandahkrishna nandahkrishna commented Feb 16, 2025

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

In recent homebrew-core PRs, we've been asking contributors to use assert_path_exists and refute_path_exists instead of assert_predicate <path>, :exist? and refute_predicate <path>, :exist? respectively. See for example: Homebrew/homebrew-core#207806 (comment), Homebrew/homebrew-core#206960 (comment), Homebrew/homebrew-core#199951, etc.

This PR makes this suggestion an autocorrectable rubocop (in some cases). Here's an example:

homebrew-core git:(master) brew style --fix empty pyinvoke wllvm pulumi
Inspecting 4 files
CCCC

Offenses:

Taps/homebrew/homebrew-core/Formula/e/empty.rb:47:39: C: [Corrected] FormulaAudit/AssertStatements: Use refute_path_exists <path_to_file> instead of refute_predicate testpath/file, :exist?
    %w[in out test.pid].each { |file| refute_predicate testpath/file, :exist? }
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Taps/homebrew/homebrew-core/Formula/p/pyinvoke.rb:38:5: C: [Corrected] FormulaAudit/AssertStatements: Use refute_path_exists <path_to_file> instead of refute_predicate testpath/"foo", :exist?, "\"pyinvoke clean\" should have deleted \"foo\""
    refute_predicate testpath/"foo", :exist?, "\"pyinvoke clean\" should have deleted \"foo\""
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Taps/homebrew/homebrew-core/Formula/p/pyinvoke.rb:39:5: C: [Corrected] FormulaAudit/AssertStatements: Use assert_path_exists <path_to_file> instead of assert_predicate testpath/"baz", :exist?, "pyinvoke should have left \"baz\""
    assert_predicate testpath/"baz", :exist?, "pyinvoke should have left \"baz\""
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Taps/homebrew/homebrew-core/Formula/p/pyinvoke.rb:41:5: C: [Corrected] FormulaAudit/AssertStatements: Use refute_path_exists <path_to_file> instead of refute_predicate testpath/"foo", :exist?, "\"pyinvoke clean-extra\" should have still deleted \"foo\""
    refute_predicate testpath/"foo", :exist?, "\"pyinvoke clean-extra\" should have still deleted \"foo\""
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Taps/homebrew/homebrew-core/Formula/p/pyinvoke.rb:42:5: C: [Corrected] FormulaAudit/AssertStatements: Use refute_path_exists <path_to_file> instead of refute_predicate testpath/"baz", :exist?, "pyinvoke clean-extra should have deleted \"baz\""
    refute_predicate testpath/"baz", :exist?, "pyinvoke clean-extra should have deleted \"baz\""
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Taps/homebrew/homebrew-core/Formula/w/wllvm.rb:30:5: C: [Corrected] FormulaAudit/AssertStatements: Use assert_path_exists <path_to_file> instead of assert_predicate testpath/".test.o", :exist?
    assert_predicate testpath/".test.o", :exist?
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Taps/homebrew/homebrew-core/Formula/w/wllvm.rb:31:5: C: [Corrected] FormulaAudit/AssertStatements: Use assert_path_exists <path_to_file> instead of assert_predicate testpath/".test.o.bc", :exist?
    assert_predicate testpath/".test.o.bc", :exist?
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Taps/homebrew/homebrew-core/Formula/w/wllvm.rb:34:5: C: [Corrected] FormulaAudit/AssertStatements: Use assert_path_exists <path_to_file> instead of assert_predicate testpath/"test.bc", :exist?
    assert_predicate testpath/"test.bc", :exist?
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Taps/homebrew/homebrew-core/Formula/p/pulumi.rb:42:5: C: [Corrected] FormulaAudit/AssertStatements: Use assert_path_exists <path_to_file> instead of assert_predicate testpath/"Pulumi.yaml", :exist?, "Project was not created"
    assert_predicate testpath/"Pulumi.yaml", :exist?, "Project was not created"
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

4 files inspected, 9 offenses detected, 9 offenses corrected

And here's the diff:

diff --git a/Formula/e/empty.rb b/Formula/e/empty.rb
index a9fb4fb875a..74a9f89bb94 100644
--- a/Formula/e/empty.rb
+++ b/Formula/e/empty.rb
@@ -44,6 +44,6 @@ class Empty < Formula
 
     system bin/"empty", "-k", File.read(testpath/"test.pid")
     sleep 1
-    %w[in out test.pid].each { |file| refute_predicate testpath/file, :exist? }
+    %w[in out test.pid].each { |file| refute_path_exists testpath/file }
   end
 end
diff --git a/Formula/p/pulumi.rb b/Formula/p/pulumi.rb
index 84366f08df9..99872b2fdc7 100644
--- a/Formula/p/pulumi.rb
+++ b/Formula/p/pulumi.rb
@@ -39,6 +39,6 @@ class Pulumi < Formula
     ENV["PULUMI_TEMPLATE_PATH"] = testpath/"templates"
     system bin/"pulumi", "new", "aws-typescript", "--generate-only",
                                                      "--force", "-y"
-    assert_predicate testpath/"Pulumi.yaml", :exist?, "Project was not created"
+    assert_path_exists testpath/"Pulumi.yaml", "Project was not created"
   end
 end
diff --git a/Formula/p/pyinvoke.rb b/Formula/p/pyinvoke.rb
index 28359fc16a2..bc6f79be022 100644
--- a/Formula/p/pyinvoke.rb
+++ b/Formula/p/pyinvoke.rb
@@ -35,10 +35,10 @@ class Pyinvoke < Formula
     (testpath/"foo"/"bar").mkpath
     (testpath/"baz").mkpath
     system bin/"invoke", "clean"
-    refute_predicate testpath/"foo", :exist?, "\"pyinvoke clean\" should have deleted \"foo\""
-    assert_predicate testpath/"baz", :exist?, "pyinvoke should have left \"baz\""
+    refute_path_exists testpath/"foo", "\"pyinvoke clean\" should have deleted \"foo\""
+    assert_path_exists testpath/"baz", "pyinvoke should have left \"baz\""
     system bin/"invoke", "clean", "--extra=baz"
-    refute_predicate testpath/"foo", :exist?, "\"pyinvoke clean-extra\" should have still deleted \"foo\""
-    refute_predicate testpath/"baz", :exist?, "pyinvoke clean-extra should have deleted \"baz\""
+    refute_path_exists testpath/"foo", "\"pyinvoke clean-extra\" should have still deleted \"foo\""
+    refute_path_exists testpath/"baz", "pyinvoke clean-extra should have deleted \"baz\""
   end
 end
diff --git a/Formula/w/wllvm.rb b/Formula/w/wllvm.rb
index 897ddd1812b..8a66d68eff7 100644
--- a/Formula/w/wllvm.rb
+++ b/Formula/w/wllvm.rb
@@ -27,10 +27,10 @@ class Wllvm < Formula
     with_env(LLVM_COMPILER: "clang") do
       system bin/"wllvm", testpath/"test.c", "-o", testpath/"test"
     end
-    assert_predicate testpath/".test.o", :exist?
-    assert_predicate testpath/".test.o.bc", :exist?
+    assert_path_exists testpath/".test.o"
+    assert_path_exists testpath/".test.o.bc"
 
     system bin/"extract-bc", testpath/"test"
-    assert_predicate testpath/"test.bc", :exist?
+    assert_path_exists testpath/"test.bc"
   end
 end

Note: I'll open a homebrew-core PR to update affected formulae if this PR makes sense. Tests currently fail because this potential PR has not been created and merged (branch: https://github.com/Homebrew/homebrew-core/tree/assert-path-rubocop).

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this! Good to merge whenever it's 🟢. Thanks!

@nandahkrishna
Copy link
Member Author

The associated homebrew-core PR has been merged and CI is 🟢 here, so I'm merging this. Thanks for the review, Mike!

@nandahkrishna nandahkrishna added this pull request to the merge queue Feb 18, 2025
Merged via the queue into master with commit 0c69873 Feb 18, 2025
34 checks passed
@nandahkrishna nandahkrishna deleted the assert-path-rubocop branch February 18, 2025 16:19
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants