Create a shared implementation for Locations and Files

[erik-krogh]

This implementation is dependent on features that have been merged to main but has not been released into a stable release yet.
The tests should start passing when the nightly build includes the required features.

I'm only using the shared implementation in Ruby, even tough I have implemented the signature in both Ruby and JS, and I'm not planning on changing that.

In JavaScript the File class have a bunch of utility predicates that don't make sense in a shared pack.
I can create a new class that extends the shared File class, but I cannot make e.g. Folder::getAFile() from the shared library return that more specific implementation.
Not unless I extend every predicate from the shared pack that can (transitively) result in a File.
And at that point there's little point in a shared implementation.

A big motivation for this work is that a shared ReDoS pack will depend a location/file implementation, and it didn't seem right to include that in the signature for ReDoS.

[aibaars]

This should probably be in a subfolder named internal , there is no reason external users should ever import this module. This is also why it was defined as a private module initially.

[erik-krogh]

A problem with that is that I want to use module FilesImpl elsewhere later, the module will not just be used for instantiating the shared File implementation.
So it feels wrong to put FilesImpl in a internal/ folder.

E.g. my shared ReDoS implementation will depend on FS::FilesSig (or something similar), so I will need the FilesImpl module when I instantiate the parameterised ReDoS implementation.
And it feels wrong to import codeql.files.internal.FilesImpl all the way from codeql.ruby.security.regexp.ExponentialBackTracking.

In the glorious future we might have many packs/modules from the shared/ folder that depend on each other, so this issue will only become bigger.

Another possible solution would be to create a ruby/ql/lib/codeql/ruby/utils folder, and that folder would be dedicated to implementations of signatures from the utils package.
(And the other languages would have a similar folder).
I kinda like that solution.
An alternative folder name would be impls/ or impl/, and that folder name could also be used elsewhere (e.g. I might create an impls/ folder under codeql.ruby.security.regexp).

Other implementations will hopefully be easier to place. E.g. the instantiations of the ReDoS library will just be in the same files that the current implementation sits in.

[hvitved]

I think it is OK for us to use internal libraries wherever we want, we just like to make sure that nobody else assumes stability of the internal ibraries.

[erik-krogh]

I still think external developers should be able to use e.g. FileSystemImpl if they develop their own shared library.

However, it's easy to later move something out of an internal/ folder, but it's tricky to move some into an internal/ folder without breaking our deprecation policy.
So I'll move the implementations into an internal/ folder.

[aibaars]

I see you changed FileSig to define proper classes instead of a bunch of predicates that are to be aliased to the matching database relations and classes that match directly to database @types.

Not sure which way is preferred. Your approach introduces an additional bunch of classes with the same hierarchy as in Make. Not sure if the optimizer can get rid of that. I think the more direct approach should result in the exact same code as the old-style hand-written implementations when the parametrised modules are instantiated.

[erik-krogh]

I think the optimizer will get rid of it once we reach the RA level, but I'm not too sure.
I'll do DCA experiments once I got a polished implementation and the compiler crash that's currently blocking me is fixed.

I like my approach (but maybe I should do something about the names).
You had both a files_ and folders_ predicate in your signature, which I've collapsed into a single getAbsolutePath() in AtContainer.
You likely chose to make those two predicate because the dbscheme has a files() and folders() relation, but that feels like an implementation detail.

[aibaars]

Let's remove the At prefix, they were there to indicate that those types mapped to an @ type.

[aibaars]

I don't see the definition of BasicFileSig anywhere, does this comment need updating?

[erik-krogh]

does this comment need updating?

Yes it does.
When I wrote that comment I had 3 modules as described, but I removed the "full implementation as a signature" module.

[hvitved]

Do these need to go into separate files, as opposed to just private modules in this file?

[erik-krogh]

As explained above.
I need the implementations of the signatures elsewhere (e.g. in the glorious future when I'm instantiating the shared ReDoS library).

So LocationsImpl as a private module won't work.

[erik-krogh]

Also we get naming conflicts due to shadowing if I put them in the same file.

I can't have a LocationsImpl that declares a class Location, and also have the file export a Location class.
That would require that we rename the classes in the signature.

[hvitved]

Not all languages may extract empty locations.

[aibaars]

That's true, not sure if this is a problem though. For those languages the EmptyLocation class would simply be empty.

[erik-krogh]

For those languages the EmptyLocation class would simply be empty.

But that would be confusing.
I'll move EmptyLocation into the language specific bits.

[erik-krogh]

I no longer think this is needed.