Skip to content
Merged
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
8 changes: 2 additions & 6 deletions shakapacker.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ Gem::Specification.new do |s|
s.add_development_dependency "rubocop"
s.add_development_dependency "rubocop-performance"

s.files = `git ls-files -z`.split("\x0").reject { |f|
f.match(%r{^(test|spec|features|tmp|node_modules|packages|coverage|Gemfile.lock|rakelib)($|/)}) ||
f.end_with?(".gem")
} + Dir.glob("sig/**/*.rbs")

s.test_files = `git ls-files -- test/*`.split("\n")
s.files = `git ls-files -z CHANGELOG.md MIT-LICENSE README.md shakapacker.gemspec lib sig`.split("\x0")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note for future contributors: git ls-files sig only includes tracked files. The old approach used Dir.glob("sig/**/*.rbs") which worked even for untracked files. If you add a new .rbs signature file, make sure to git add it before building the gem, otherwise it will be silently excluded from the published package.

s.test_files = []
end
32 changes: 32 additions & 0 deletions spec/shakapacker/gemspec_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,46 @@
expect(node_modules_files).to be_empty
end

it "excludes JavaScript package source" do
package_files = gemspec.files.select { |f| f.start_with?("package/") }
expect(package_files).to be_empty
end

it "excludes repository-only documentation and test files" do
excluded_files = gemspec.files.select { |f| f.start_with?("docs/", "test/") }
expect(excluded_files).to be_empty
end

it "includes lib directory" do
lib_files = gemspec.files.select { |f| f.start_with?("lib/") }
expect(lib_files).not_to be_empty
end

it "includes install assets needed by shakapacker:install" do
expect(gemspec.files).to include(
"lib/install/template.rb",
"lib/install/application.js",
"lib/install/package.json",
"lib/install/config/shakapacker.yml",
"lib/install/bin/shakapacker",
"lib/install/bin/shakapacker-dev-server"
"lib/install/bin/shakapacker",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing comma causes silent string concatenation in test

High Severity

A missing comma after "lib/install/bin/shakapacker-dev-server" on line 47 causes Ruby to implicitly concatenate it with the next string literal "lib/install/bin/shakapacker", producing the nonsensical path "lib/install/bin/shakapacker-dev-serverlib/install/bin/shakapacker". This appears to be a copy-paste error — lines 46–47 duplicate lines 48–49. Additionally, the include( opened on line 41 is never closed with ), which is a syntax error.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ce32605. Configure here.

"lib/install/bin/shakapacker-dev-server",
Comment on lines +47 to +49

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: missing comma causes implicit string concatenation + duplicate entries

Line 47 is missing a trailing comma, so Ruby silently concatenates the two adjacent string literals into one string:

"lib/install/bin/shakapacker-dev-server" "lib/install/bin/shakapacker"
# => "lib/install/bin/shakapacker-dev-serverlib/install/bin/shakapacker"

That concatenated path doesn't exist, so the assertion is checking for a nonexistent file. Lines 48 and 49 also duplicate the two entries that should have appeared here.

Suggested change
"lib/install/bin/shakapacker-dev-server"
"lib/install/bin/shakapacker",
"lib/install/bin/shakapacker-dev-server",
"lib/install/bin/shakapacker",
"lib/install/bin/shakapacker-dev-server",

"lib/install/config/rspack/rspack.config.js",
"lib/install/config/rspack/rspack.config.ts",
"lib/install/config/webpack/webpack.config.js",
"lib/install/config/webpack/webpack.config.ts"
end
Comment on lines +40 to +54

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P0 Two syntax bugs make this entire example unparseable at runtime. First, the missing comma after "lib/install/bin/shakapacker-dev-server" on line 47 causes Ruby's implicit adjacent-literal concatenation, turning that line and the next into a single fabricated path "lib/install/bin/shakapacker-dev-serverlib/install/bin/shakapacker" that will never match a real file. Second, the opening include( on line 41 is never closed with ) before end, so Ruby raises SyntaxError: unexpected keyword_end, expecting ')' and the entire spec file fails to load. Together these defeat the stated validation step bundle exec rspec spec/shakapacker/gemspec_spec.rb.

Suggested change
it "includes install assets needed by shakapacker:install" do
expect(gemspec.files).to include(
"lib/install/template.rb",
"lib/install/application.js",
"lib/install/package.json",
"lib/install/config/shakapacker.yml",
"lib/install/bin/shakapacker",
"lib/install/bin/shakapacker-dev-server"
"lib/install/bin/shakapacker",
"lib/install/bin/shakapacker-dev-server",
"lib/install/config/rspack/rspack.config.js",
"lib/install/config/rspack/rspack.config.ts",
"lib/install/config/webpack/webpack.config.js",
"lib/install/config/webpack/webpack.config.ts"
end
it "includes install assets needed by shakapacker:install" do
expect(gemspec.files).to include(
"lib/install/template.rb",
"lib/install/application.js",
"lib/install/package.json",
"lib/install/config/shakapacker.yml",
"lib/install/bin/shakapacker",
"lib/install/bin/shakapacker-dev-server",
"lib/install/config/rspack/rspack.config.js",
"lib/install/config/rspack/rspack.config.ts",
"lib/install/config/webpack/webpack.config.js",
"lib/install/config/webpack/webpack.config.ts"
)
end

Comment on lines +53 to +54

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: missing closing ) — syntax error

The include( call opened on line 41 is never closed before end. This is a SyntaxError that prevents the entire spec file from loading, so none of the tests in this file will run.

Suggested change
"lib/install/config/webpack/webpack.config.ts"
end
"lib/install/config/webpack/webpack.config.ts"
)
end

Comment on lines +40 to +54

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This it block has two bugs that make it provide no real coverage:

Bug 1 — Missing closing ) for include( (line 54)
The include( opened on line 41 is never closed with ). Ruby sees the end on line 54 and raises a SyntaxError — the block never runs.

Bug 2 — Missing comma causes implicit string concatenation (line 47)

"lib/install/bin/shakapacker-dev-server"   # ← no trailing comma
"lib/install/bin/shakapacker",

Ruby silently concatenates adjacent string literals at parse time, producing "lib/install/bin/shakapacker-dev-servershakapacker". The test would be asserting the existence of a path that never exists.

Bug 3 — Duplicate entries
"lib/install/bin/shakapacker" and "lib/install/bin/shakapacker-dev-server" each appear twice.

Suggested change
it "includes install assets needed by shakapacker:install" do
expect(gemspec.files).to include(
"lib/install/template.rb",
"lib/install/application.js",
"lib/install/package.json",
"lib/install/config/shakapacker.yml",
"lib/install/bin/shakapacker",
"lib/install/bin/shakapacker-dev-server"
"lib/install/bin/shakapacker",
"lib/install/bin/shakapacker-dev-server",
"lib/install/config/rspack/rspack.config.js",
"lib/install/config/rspack/rspack.config.ts",
"lib/install/config/webpack/webpack.config.js",
"lib/install/config/webpack/webpack.config.ts"
end
it "includes install assets needed by shakapacker:install" do
expect(gemspec.files).to include(
"lib/install/template.rb",
"lib/install/application.js",
"lib/install/package.json",
"lib/install/config/shakapacker.yml",
"lib/install/bin/shakapacker",
"lib/install/bin/shakapacker-dev-server",
"lib/install/config/rspack/rspack.config.js",
"lib/install/config/rspack/rspack.config.ts",
"lib/install/config/webpack/webpack.config.js",
"lib/install/config/webpack/webpack.config.ts"
)
end


it "includes RBS type signatures" do
rbs_files = gemspec.files.select { |f| f.end_with?(".rbs") }
expect(rbs_files).not_to be_empty
end
end

describe "s.test_files" do
it "is empty" do
expect(gemspec.test_files).to be_empty

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

test_files is deprecated since RubyGems 2.0 and always returns [] in modern environments, so this spec cannot fail in practice. It's a harmless sanity-check, but consider whether it's worth carrying the maintenance weight. If kept, a brief comment explaining why it's explicitly set to [] (to silence the deprecation warning from the previous git ls-files -- test/* assignment) would help future readers.

end
end
end
Loading