feat(api): Accept structured imports [LNG-288] #989
Conversation
LNG-288 Accept structured imports in compiler API
In aqua-api, accept imports as the following structure: While compiling a file, use imports from all entries that are prefixes for this file path (prioritize longer prefixes). |
| /** | ||
| * Imports resolution configuration. | ||
| */ | ||
| final case class Imports( |
There was a problem hiding this comment.
there is two same classes with the same structure. Maybe merge it somehow?
There was a problem hiding this comment.
There are two of them so that api and io packages have clear dependencies.
If we put Imports to api, io would have to depend on api and this would create cyclic dependency. Also Imports define the logic of resolving imports and api does not feel like the proper place for it.
If we put Imports to io, then api would have to expose internal io type in interface. This does not feel right too.
So those two classes exist to decouple interface representation and representation for internal logic. If one of them changes, we would need to change only the conversion function. But now they are the same, yes.
| } | ||
|
|
||
| // Get all files if the path is a directory or this path otherwise | ||
| // TODO: Test it or remove it. It is not used anywhere |
There was a problem hiding this comment.
but it was used in AquaFileSources
There was a problem hiding this comment.
Yes. I meant folder walking functionality. Right now it is used on files only. Rephrased comment.
|
|
||
| // Get all files if the path is a directory or this path otherwise | ||
| // TODO: Test it or remove it. It is not used anywhere | ||
| override def listAqua(folder: Path): EitherT[F, AquaFileError, Chain[Path]] = |
There was a problem hiding this comment.
what if folder will be not a folder, but a file?
There was a problem hiding this comment.
If tests pass, it means this works, yes? Also from the source code it follows that the path itself is returned in stream too, independent of whether it is a folder or not:
https://github.com/typelevel/fs2/blob/aba387fec4841d575c7a66d65594f893b728b6ec/io/shared/src/main/scala/fs2/io/file/Files.scala#L504
| settings.filter { case (prefix, _) => | ||
| from.startsWith(prefix) | ||
| }.maxByOption { case (prefix, _) => | ||
| prefix.toString.length |
There was a problem hiding this comment.
Shouldn't the longest prefix be compared by the number of folders, not characters?
There was a problem hiding this comment.
If two paths are prefixes of from, the longer one have at least the same amount of folder as the shorter one.
Also, they can have the same amount of folders, example:
from = /a/b/ccc/d.aqua, p1 = /a/b/c, p2 = /a/b/cc, p1 and p2 have the same amount of folders. So we should pick p2 because it is a longer prefix in terms of characters.
| } | ||
|
|
||
| // Transform each inner string into an array | ||
| return Object.fromEntries( |
| } | ||
|
|
||
| // Get all files if the path is a directory or this path otherwise | ||
| // TODO: Test it or refactor. Right now it is used on single files only |
There was a problem hiding this comment.
it is used with a directory in integration tests
Description
Allow specifying imports based on compiled file path prefix and import prefix to resolve issues with transitive dependencies (no way to have different versions of a transitive dependency).
Proposed Changes
Accept imports in form
Implementation Details
Refactor
AquaFileSources.Checklist
Reviewer Checklist