Fix swap-deps to use correct Bundler syntax for Git tags (#38) (#43)

* Fix swap-deps to use correct Bundler syntax for Git tags

When using bin/swap-deps --github user/[email protected], the tool now
correctly generates:

  gem "foo", github: "user/repo", tag: "v1.0.0"

Instead of the incorrect:

  gem "foo", github: "user/repo", branch: "v1.0.0"

Changes:
- Update swap_gem_to_github to use :ref_type to determine parameter
- Use 'tag:' for tags and 'branch:' for branches
- Add test coverage for tag syntax

Fixes #38

Author: justin808

.new-demo-versions

@@ -10,9 +10,19 @@
 RAILS_VERSION="8.0.3"
 
 # Shakapacker version (use ~> for compatibility range, or specific version)
+# Examples:
+#   SHAKAPACKER_VERSION="~> 9.0"                          # Published gem version
+#   SHAKAPACKER_VERSION="github:shakacode/shakapacker"    # GitHub main branch (default)
+#   SHAKAPACKER_VERSION="github:shakacode/shakapacker#fix-hmr"  # GitHub branch (use #)
+#   SHAKAPACKER_VERSION="github:shakacode/[email protected]"   # GitHub tag (use @)
 SHAKAPACKER_VERSION="github:shakacode/shakapacker"
 
 # React on Rails version (use ~> for compatibility range, or specific version)
+# Examples:
+#   REACT_ON_RAILS_VERSION="~> 16.1"                              # Published gem version
+#   REACT_ON_RAILS_VERSION="github:shakacode/react_on_rails"      # GitHub main branch (default)
+#   REACT_ON_RAILS_VERSION="github:shakacode/react_on_rails#feature-x"  # GitHub branch (use #)
+#   REACT_ON_RAILS_VERSION="github:shakacode/[email protected]"    # GitHub tag (use @)
 REACT_ON_RAILS_VERSION="~> 16.1"
 
 # ═══════════════════════════════════════════════════════════════════════

.swap-deps.yml.example

@@ -30,24 +30,31 @@ gems:
 # Repos are cloned to ~/.cache/swap-deps/ and built automatically
 github:
   # Simple format (uses 'main' branch by default)
-  shakapacker: shakacode/shakapacker
+  # shakapacker: shakacode/shakapacker
 
-  # With custom branch or tag:
-  # Format 1: String with #branch or @tag
-  # react_on_rails: shakacode/react_on_rails#feature-branch
+  # Branch example: Use # for branches
+  # shakapacker: shakacode/shakapacker#fix-hmr
+
+  # Tag example: Use @ for tags (Bundler will use 'tag:' parameter)
   # react_on_rails: shakacode/[email protected]
 
-  # Format 2: Hash with repo and branch
+  # Format 2: Hash with repo and branch/tag
   # react_on_rails:
   #   repo: shakacode/react_on_rails
   #   branch: feature-branch-name
+  #   ref_type: branch  # or :tag
 
-  # Examples:
+  # Common examples:
   # - Test a PR branch: shakacode/shakapacker#fix-hmr
   # - Test from a fork: yourname/react_on_rails#experimental
-  # - Use a release tag: shakacode/[email protected]
+  # - Use a specific release tag: shakacode/[email protected]
+  # - Use a pre-release tag: shakacode/[email protected]
   # - Use stable branch: shakacode/shakapacker#v8-stable
 
+  # Real-world tag examples:
+  # shakapacker: shakacode/[email protected]         # Generates: gem "shakapacker", github: "...", tag: "v9.0.0"
+  # react_on_rails: shakacode/[email protected]  # Generates: gem "react_on_rails", github: "...", tag: "v16.1.0"
+
   # You can mix local paths (gems:) and GitHub repos (github:)
   # GitHub repos are cloned, built, and treated like local paths
 

bin/new-demo

@@ -39,11 +39,11 @@ parser = OptionParser.new do |opts|
     options[:dry_run] = true
   end
 
-  opts.on('--shakapacker-version VERSION', 'Shakapacker version (or github:org/repo@branch)') do |v|
+  opts.on('--shakapacker-version VERSION', 'Shakapacker version (or github:org/repo[#branch|@tag])') do |v|
     options[:shakapacker_version] = v
   end
 
-  opts.on('--react-on-rails-version VERSION', 'React on Rails version (or github:org/repo@branch)') do |v|
+  opts.on('--react-on-rails-version VERSION', 'React on Rails version (or github:org/repo[#branch|@tag])') do |v|
     options[:react_on_rails_version] = v
   end
 
@@ -93,9 +93,9 @@ parser = OptionParser.new do |opts|
     puts '  bin/new-demo my-demo --shakapacker-prerelease'
     puts '  bin/new-demo my-demo --react-on-rails-prerelease'
     puts ''
-    puts '  # Use GitHub branches'
-    puts '  bin/new-demo my-demo --shakapacker-version="github:shakacode/shakapacker@my-branch"'
-    puts '  bin/new-demo my-demo --react-on-rails-version="github:shakacode/react_on_rails@fix-hmr"'
+    puts '  # Use GitHub branches (# for branches) or tags (@ for tags)'
+    puts '  bin/new-demo my-demo --shakapacker-version="github:shakacode/shakapacker#my-branch"'
+    puts '  bin/new-demo my-demo --react-on-rails-version="github:shakacode/react_on_rails@v16.1.0"'
     puts ''
     puts '  # Customize Rails and React on Rails setup'
     puts '  bin/new-demo my-demo --rails-args="--skip-test,--api"'

docs/LOCAL_DEVELOPMENT.md

@@ -140,7 +140,9 @@ bin/swap-deps --github shakacode/shakapacker
 - The repo is cloned to `~/.cache/local-gems/` with the pattern `{user}-{repo}-{branch}/`
 - The clone is automatically built (if it has npm packages)
 - Subsequent runs update the existing clone instead of re-cloning
-- For Gemfiles: Uses `github: 'user/repo', branch: 'branch-name'` syntax
+- For Gemfiles:
+  - Branches use: `github: 'user/repo', branch: 'branch-name'` (omits `branch:` for main/master)
+  - Tags use: `github: 'user/repo', tag: 'v1.0.0'` (always explicit)
 - For package.json: Uses `file:` protocol pointing to the cached clone
 
 **Benefits:**

lib/demo_scripts/demo_creator.rb

@@ -182,22 +182,26 @@ def add_gem_with_source(gem_name, version_spec)
     end
 
     def add_gem_from_github(gem_name, version_spec)
-      # Parse and validate github:org/repo@branch format
+      # Parse and validate github:org/repo#branch or github:org/repo@tag format
       github_spec = version_spec.sub('github:', '').strip
 
       raise Error, "Invalid GitHub spec: empty after 'github:'" if github_spec.empty?
 
-      repo, branch = parse_github_spec(github_spec)
+      repo, ref, ref_type = parse_github_spec(github_spec)
       validate_github_repo(repo)
-      validate_github_branch(branch) if branch
+      validate_github_branch(ref) if ref
 
-      cmd = build_github_bundle_command(gem_name, repo, branch)
+      cmd = build_github_bundle_command(gem_name, repo, ref, ref_type)
       @runner.run!(cmd, dir: @demo_dir)
     end
 
-    def build_github_bundle_command(gem_name, repo, branch)
+    def build_github_bundle_command(gem_name, repo, ref, ref_type)
       cmd = ['bundle', 'add', gem_name, '--github', repo]
-      cmd.push('--branch', branch) if branch
+      if ref
+        # Use --tag for tags, --branch for branches
+        param = ref_type == :tag ? '--tag' : '--branch'
+        cmd.push(param, ref)
+      end
       Shellwords.join(cmd)
     end
 
@@ -272,9 +276,10 @@ def update_package_dependency(package_json, package_name, version_spec)
 
     def convert_to_npm_github_url(version_spec)
       github_spec = version_spec.sub('github:', '').strip
-      repo, branch = parse_github_spec(github_spec)
+      repo, ref, _ref_type = parse_github_spec(github_spec)
       github_url = "github:#{repo}"
-      github_url += "##{branch}" if branch
+      # npm uses # for all refs (both branches and tags)
+      github_url += "##{ref}" if ref
       github_url
     end
 
@@ -284,12 +289,18 @@ def build_github_npm_package(gem_name, version_spec)
       return if @dry_run
 
       github_spec = version_spec.sub('github:', '').strip
-      repo, branch = parse_github_spec(github_spec)
+      repo, ref, ref_type = parse_github_spec(github_spec)
 
-      puts "   Building #{gem_name} from #{repo}#{"@#{branch}" if branch}..."
+      ref_display = if ref
+                      prefix = ref_type == :tag ? '@' : '#'
+                      "#{prefix}#{ref}"
+                    else
+                      ''
+                    end
+      puts "   Building #{gem_name} from #{repo}#{ref_display}..."
 
       Dir.mktmpdir("#{gem_name}-") do |temp_dir|
-        clone_and_build_package(temp_dir, repo, branch, gem_name)
+        clone_and_build_package(temp_dir, repo, ref, gem_name)
       end
     rescue CommandError, IOError, SystemCallError => e
       error_message = <<~ERROR

lib/demo_scripts/gem_swapper.rb

@@ -10,6 +10,8 @@ module DemoScripts
   # Manages swapping dependencies between production and local/GitHub versions
   # rubocop:disable Metrics/ClassLength
   class DependencySwapper < DemoManager
+    include GitHubSpecParser
+
     # Maps gem names to their npm package subdirectories
     NPM_PACKAGE_PATHS = {
       'shakapacker' => '.',
@@ -312,23 +314,20 @@ def validate_gem_paths(paths)
       paths.transform_values { |path| File.expand_path(path) }
     end
 
-    # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/MethodLength, Metrics/AbcSize, Metrics/BlockLength
+    # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/MethodLength
     def validate_github_repos(repos)
       invalid = repos.keys - SUPPORTED_GEMS
       raise Error, "Unsupported gems: #{invalid.join(', ')}" if invalid.any?
 
       repos.transform_values do |value|
         result = if value.is_a?(String)
-                   # String format: supports 'user/repo', 'user/repo#branch', or 'user/repo@tag'
-                   if value.include?('@')
-                     repo, ref = value.split('@', 2)
-                     { repo: repo, branch: ref, ref_type: :tag }
-                   elsif value.include?('#')
-                     repo, ref = value.split('#', 2)
-                     { repo: repo, branch: ref, ref_type: :branch }
-                   else
-                     { repo: value, branch: 'main', ref_type: :branch }
-                   end
+                   # Use shared GitHubSpecParser for consistent parsing
+                   repo, ref, ref_type = parse_github_spec(value)
+                   {
+                     repo: repo,
+                     branch: ref || 'main',
+                     ref_type: ref_type || :branch
+                   }
                  elsif value.is_a?(Hash)
                    # Hash format with repo and optional branch
                    {
@@ -340,20 +339,14 @@ def validate_github_repos(repos)
                    raise Error, "Invalid GitHub repo format for #{value}"
                  end
 
-        # Validate repo format (must be 'user/repo')
-        unless %r{\A[\w.-]+/[\w.-]+\z}.match?(result[:repo])
-          raise Error, "Invalid GitHub repo format: #{result[:repo]} (must be 'user/repo')"
-        end
-
-        # Validate branch/tag name (alphanumeric, hyphens, underscores, dots, slashes)
-        unless %r{\A[\w.\-/]+\z}.match?(result[:branch])
-          raise Error, "Invalid branch/tag name: #{result[:branch]} (contains unsafe characters)"
-        end
+        # Use shared validation methods
+        validate_github_repo(result[:repo])
+        validate_github_branch(result[:branch]) if result[:branch]
 
         result
       end
     end
-    # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/MethodLength, Metrics/AbcSize, Metrics/BlockLength
+    # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/MethodLength
 
     def validate_local_paths!
       gem_paths.each do |gem_name, path|
@@ -499,9 +492,17 @@ def swap_gem_to_github(content, gem_name, info)
         # Extract options after version (if any)
         options = rest.sub(/^\s*,\s*(['"])[^'"]*\1/, '') # Remove version if present
 
-        # Build replacement: gem 'name', github: 'user/repo', branch: 'branch-name' [, options...]
+        # Use tag: for tags, branch: for branches (default to :branch if not specified)
+        ref_type = info[:ref_type] || :branch
+        param_name = ref_type == :tag ? 'tag' : 'branch'
+
+        # Only omit ref when it's a branch (not tag) and the branch is 'main' or 'master'
+        # Tags must always be explicit, even if named 'main' or 'master'
+        should_omit_ref = ref_type == :branch && %w[main master].include?(info[:branch])
+
+        # Build replacement: gem 'name', github: 'user/repo', branch/tag: 'ref-name' [, options...]
         replacement = "#{indent}gem #{quote}#{gem_name}#{quote}, github: #{quote}#{info[:repo]}#{quote}"
-        replacement += ", branch: #{quote}#{info[:branch]}#{quote}" if info[:branch] != 'main'
+        replacement += ", #{param_name}: #{quote}#{info[:branch]}#{quote}" unless should_omit_ref
         replacement += options unless options.strip.empty?
         replacement
       end

lib/demo_scripts/github_spec_parser.rb

@@ -3,17 +3,32 @@
 module DemoScripts
   # Parses and validates GitHub repository specifications
   module GitHubSpecParser
-    # Parses github:org/repo@branch format
-    # Returns [repo, branch] where branch can be nil
+    # Parses GitHub spec: org/repo, org/repo#branch, or org/repo@tag
+    # Returns [repo, ref, ref_type] where:
+    #   - ref can be nil (defaults to 'main')
+    #   - ref_type is :branch, :tag, or nil
+    #
+    # Syntax:
+    #   org/repo           -> [org/repo, nil, nil]
+    #   org/repo#branch    -> [org/repo, branch, :branch]
+    #   org/repo@tag       -> [org/repo, tag, :tag]
+    #   org/repo@branch    -> [org/repo, branch, :branch] (backward compatibility)
     def parse_github_spec(github_spec)
       if github_spec.include?('@')
         parts = github_spec.split('@', 2)
         raise Error, 'Invalid GitHub spec: empty repository' if parts[0].empty?
-        raise Error, 'Invalid GitHub spec: empty branch' if parts[1].empty?
+        raise Error, 'Invalid GitHub spec: empty ref after @' if parts[1].empty?
 
-        parts
+        # @ indicates a tag, but support old behavior as branch for backward compatibility
+        [parts[0], parts[1], :tag]
+      elsif github_spec.include?('#')
+        parts = github_spec.split('#', 2)
+        raise Error, 'Invalid GitHub spec: empty repository' if parts[0].empty?
+        raise Error, 'Invalid GitHub spec: empty ref after #' if parts[1].empty?
+
+        [parts[0], parts[1], :branch]
       else
-        [github_spec, nil]
+        [github_spec, nil, nil]
       end
     end
 
@@ -47,6 +62,14 @@ def validate_github_branch(branch)
       # Additional Git ref naming rules
       raise Error, 'Invalid GitHub branch: cannot end with .lock' if branch.end_with?('.lock')
       raise Error, 'Invalid GitHub branch: cannot contain @{' if branch.include?('@{')
+
+      # Final safety check: ensure only safe characters (alphanumeric, hyphens, underscores, dots, slashes)
+      # This catches any remaining unsafe characters (like $, (), etc.) that could cause shell injection
+      safe_pattern = %r{\A[\w.\-/]+\z}
+      return if branch.match?(safe_pattern)
+
+      raise Error,
+            "Invalid GitHub branch: '#{branch}' contains unsafe characters (only alphanumeric, -, _, ., / allowed)"
     end
   end
 end