Only strip elidable extensions to make short forms

Otherwise we'll erroneously think that 'descriptor' ('descriptor.rb')
is a loaded feature when somebody requires 'descriptor.pb' ('descriptor.pb.rb'),
when in fact those are two completely unrelated files.

Elidable extensions include Ruby '.rb' files and shared libraries.

Ruby does some slightly magical things in 'require', e.g., letting
you refer to shared libraries by extensions that _would_ have been
valid on another platform ('libgit2.dll' works on Linux!) which would
be difficult to replicate precisely without a clear spec, so this change
makes the simplifying assumption that people won't use these extensions
for anything other than shared libraries.

Author: jeffparsons

lib/bootsnap/load_path_cache/loaded_features_index.rb

@@ -40,7 +40,7 @@ def initialize
             # /a/b/lib/my/foo.rb
             #          ^^^^^^^^^
             short = feat[(lpe.length + 1)..-1]
-            stripped = strip_extension(short)
+            stripped = strip_extension_if_elidable(short)
             @lfi[short] = hash
             @lfi[stripped] = hash
           end
@@ -94,13 +94,14 @@ def register(short, long = nil)
 
         hash = long.hash
 
-        # do we have 'bundler' or 'bundler.rb'?
-        altname = if File.extname(short) != ''
-          # strip the path from 'bundler.rb' -> 'bundler'
-          strip_extension(short)
+        # Do we have a filename with an elidable extension, e.g.,
+        # 'bundler.rb', or 'libgit2.so'?
+        altname = if is_extension_elidable(short)
+          # Strip the extension off, e.g. 'bundler.rb' -> 'bundler'.
+          strip_extension_if_elidable(short)
         elsif long && (ext = File.extname(long))
-          # get the extension from the expanded path if given
-          # 'bundler' + '.rb'
+          # We already know the extension of the actual file this
+          # resolves to, so put that back on.
           short + ext
         end
 
@@ -117,8 +118,30 @@ def register(short, long = nil)
       STRIP_EXTENSION = /\.[^.]*?$/
       private_constant(:STRIP_EXTENSION)
 
-      def strip_extension(f)
-        f.sub(STRIP_EXTENSION, '')
+      # Might Ruby automatically search for this extension if
+      # someone tries to 'require' the file without it? E.g. Ruby
+      # will implicitly try 'x.rb' if you ask for 'x'.
+      #
+      # This is complex and platform-dependent, and the Ruby docs are a little
+      # handwavy about what will be tried when and in what order.
+      # So optimistically pretend that all known elidable extensions
+      # will be tried on all platforms, and that people are unlikely
+      # to name files in a way that assumes otherwise.
+      # (E.g. It's unlikely that someone will know that their code
+      # will _never_ run on MacOS, and therefore think they can get away
+      # with callling a Ruby file 'x.dylib.rb' and then requiring it as 'x.dylib'.)
+      #
+      # See <https://ruby-doc.org/core-2.6.4/Kernel.html#method-i-require>.
+      def is_extension_elidable(f)
+        f.end_with?('.rb', '.so', '.o', '.dll', '.dylib')
+      end
+
+      def strip_extension_if_elidable(f)
+        if is_extension_elidable(f)
+          f.sub(STRIP_EXTENSION, '')
+        else
+          f
+        end
       end
     end
   end

test/load_path_cache/loaded_features_index_test.rb

@@ -41,6 +41,40 @@ def test_infer_base_from_ext
         refute(@index.key?('foo'))
       end
 
+      def test_only_strip_elidable_ext
+        # It is only valid to strip a '.rb' or shared library extension from the
+        # end of a filename, not anything else.
+        #
+        # E.g. 'descriptor.pb.rb' if required via 'descriptor.pb'
+        # should never be shortened to merely 'descriptor'!
+        refute(@index.key?('descriptor.pb'))
+        refute(@index.key?('descriptor.pb.rb'))
+        refute(@index.key?('descriptor.rb'))
+        refute(@index.key?('descriptor'))
+        refute(@index.key?('foo'))
+        @index.register('descriptor.pb.rb') {}
+        assert(@index.key?('descriptor.pb'))
+        assert(@index.key?('descriptor.pb.rb'))
+        refute(@index.key?('descriptor.rb'))
+        refute(@index.key?('descriptor'))
+        refute(@index.key?('foo'))
+      end
+
+      def test_shared_library_ext_considered_elidable
+        # Check that '.dylib' (token shared library extension) is treated as elidable,
+        # and doesn't get mixed up with Ruby '.rb' files.
+        refute(@index.key?('libgit2.dylib'))
+        refute(@index.key?('libgit2.dylib.rb'))
+        refute(@index.key?('descriptor.rb'))
+        refute(@index.key?('descriptor'))
+        refute(@index.key?('foo'))
+        @index.register('libgit2.dylib') {}
+        assert(@index.key?('libgit2.dylib'))
+        refute(@index.key?('libgit2.dylib.rb'))
+        refute(@index.key?('libgit2.rb'))
+        refute(@index.key?('foo'))
+      end
+
       def test_cannot_infer_ext_from_base # Current limitation
         refute(@index.key?('bundler'))
         refute(@index.key?('bundler.rb'))