Repository navigation
Support explicit package library entry points - #14
Merged
Merged
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Reviewer's GuidePackages can now declare a safe, explicit Sequence diagram for explicit package library resolutionsequenceDiagram
participant Importer
participant Runtime
participant Package
participant EntryPoint
Importer->>Runtime: sys_resolve_module(package)
Runtime->>Package: pkg_declared_lib(package_dir, declared)
Package-->>Runtime: declared lib path or no declaration
alt valid declared lib
Runtime->>EntryPoint: pkg_candidate(package_dir/lib)
EntryPoint-->>Runtime: resolved module
else declared lib invalid or missing
Runtime-->>Importer: import error
else no lib declaration
Runtime->>Package: pkg_candidate(main.fl)
Runtime->>Package: pkg_candidate(package/package.fl)
Runtime->>Package: pkg_candidate(package.fl)
Package-->>Runtime: legacy module match
end
Runtime-->>Importer: module path or failure
Flow diagram for package entry-point selectionflowchart TD
A[Import package name] --> B[Locate installed package root]
B --> C{"[package] lib declared?"}
C -->|Yes| D[Validate relative .fl path inside package root]
D --> E{Target exists?}
E -->|Yes| F[Use declared public module]
E -->|No| G[Reject import]
D -->|Invalid| G
C -->|No| H[Try main.fl]
H --> I[Try same-named file]
I --> J[Try flat package file]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/runtime/sys.c" line_range="571-572" />
<code_context>
- need2,
- "%s.fl",
+ bool has_declared_lib = false;
+ found = pkg_declared_lib(
+ dir_main, &has_declared_lib);
+ if (!has_declared_lib) {
+ size_t need = strlen(dir_main) + 9;
</code_context>
<issue_to_address>
**Invalid package entry point falls through**
When a declared `lib` target is missing or invalid and a same-named standard-library module or sibling `.fl` file is available, `sys_resolve_module` continues lookup, so the standard-library module or sibling file loads instead of the package import failing for its invalid entry point.
Stop resolution and report failure when a declared `lib` target cannot be resolved.
Also at `rust/flintlang/c-src/src/runtime/sys.c:571-573`, `src/runtime/sys.c:573`.
</issue_to_address>
### Comment 2
<location path="src/runtime/sys.c" line_range="497-499" />
<code_context>
+ if (*tail != '\0' && *tail != '#')
+ break;
+ *end = '\0';
+ size_t len = strlen(p);
+ if (len < 4 || strcmp(p + len - 3, ".fl") != 0 ||
+ p[0] == '/' || strchr(p, '\\') != NULL)
+ break;
+ bool unsafe = false;
</code_context>
<issue_to_address>
**Hidden library files go missing**
When a package declares its library under a dot-prefixed file or directory, the resolver accepts the path, but package installation skips dot-prefixed entries, so the declared target is absent and importing the package fails.
Ensure declared library paths are copied during installation, or reject paths that installation will skip.
Also at `rust/flintlang/c-src/src/runtime/sys.c:497-499`, `src/runtime/sys.c:571-572`.
</issue_to_address>
### Comment 3
<location path="src/runtime/sys.c" line_range="571-572" />
<code_context>
- need2,
- "%s.fl",
+ bool has_declared_lib = false;
+ found = pkg_declared_lib(
+ dir_main, &has_declared_lib);
+ if (!has_declared_lib) {
+ size_t need = strlen(dir_main) + 9;
</code_context>
<issue_to_address>
**Custom packages lack import hints**
When a user references a name from a package with a declared `lib` entry point but omits its import, `sys_module_file_exists` still checks only `flint_modules/<name>/main.fl`, so the undefined-name diagnostic does not recognize packages whose only entry point is declared with `lib`. Users who forget an import for such a package get no “forgot the import” hint.
Update `sys_module_file_exists` to use the same declared-entry-point lookup as the module resolver.
Also at `rust/flintlang/c-src/src/runtime/sys.c:571-572`.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: src/runtime/sys.c:572, src/runtime/sys.c:499
Comment on lines
+571
to
+572
| found = pkg_declared_lib( | ||
| dir_main, &has_declared_lib); |
Contributor
There was a problem hiding this comment.
🟡 Medium · Invalid package entry point falls through
When a declared lib target is missing or invalid and a same-named standard-library module or sibling .fl file is available, sys_resolve_module continues lookup, so the standard-library module or sibling file loads instead of the package import failing for its invalid entry point.
Stop resolution and report failure when a declared lib target cannot be resolved.
Also at rust/flintlang/c-src/src/runtime/sys.c:571-573, src/runtime/sys.c:573.
Prompt for AI agents
In `src/runtime/sys.c` at lines 571-572:
**Invalid package entry point falls through**
When a declared `lib` target is missing or invalid and a same-named standard-library module or sibling `.fl` file is available, `sys_resolve_module` continues lookup, so the standard-library module or sibling file loads instead of the package import failing for its invalid entry point.
Stop resolution and report failure when a declared `lib` target cannot be resolved.
Also at `rust/flintlang/c-src/src/runtime/sys.c:571-573`, `src/runtime/sys.c:573`.
Comment on lines
+497
to
+499
| size_t len = strlen(p); | ||
| if (len < 4 || strcmp(p + len - 3, ".fl") != 0 || | ||
| p[0] == '/' || strchr(p, '\\') != NULL) |
Contributor
There was a problem hiding this comment.
🟡 Medium · Hidden library files go missing
When a package declares its library under a dot-prefixed file or directory, the resolver accepts the path, but package installation skips dot-prefixed entries, so the declared target is absent and importing the package fails.
Ensure declared library paths are copied during installation, or reject paths that installation will skip.
Also at rust/flintlang/c-src/src/runtime/sys.c:497-499, src/runtime/sys.c:571-572.
Prompt for AI agents
In `src/runtime/sys.c` at lines 497-499:
**Hidden library files go missing**
When a package declares its library under a dot-prefixed file or directory, the resolver accepts the path, but package installation skips dot-prefixed entries, so the declared target is absent and importing the package fails.
Ensure declared library paths are copied during installation, or reject paths that installation will skip.
Also at `rust/flintlang/c-src/src/runtime/sys.c:497-499`, `src/runtime/sys.c:571-572`.
Comment on lines
+571
to
+572
| found = pkg_declared_lib( | ||
| dir_main, &has_declared_lib); |
Contributor
There was a problem hiding this comment.
⚪ Low · Custom packages lack import hints
When a user references a name from a package with a declared lib entry point but omits its import, sys_module_file_exists still checks only flint_modules/<name>/main.fl, so the undefined-name diagnostic does not recognize packages whose only entry point is declared with lib. Users who forget an import for such a package get no “forgot the import” hint.
Update sys_module_file_exists to use the same declared-entry-point lookup as the module resolver.
Also at rust/flintlang/c-src/src/runtime/sys.c:571-572.
Prompt for AI agents
In `src/runtime/sys.c` at lines 571-572:
**Custom packages lack import hints**
When a user references a name from a package with a declared `lib` entry point but omits its import, `sys_module_file_exists` still checks only `flint_modules/<name>/main.fl`, so the undefined-name diagnostic does not recognize packages whose only entry point is declared with `lib`. Users who forget an import for such a package get no “forgot the import” hint.
Update `sys_module_file_exists` to use the same declared-entry-point lookup as the module resolver.
Also at `rust/flintlang/c-src/src/runtime/sys.c:571-572`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
[package] lib = "src/api.fl"to select a package's public import entry point.main.fl, same-named-file, and flat-file conventions whenlibis absent.Validation
Summary by Sourcery
Support explicit, package-relative library entry points while preserving compatibility for packages without a
libdeclaration.New Features:
libfield.Bug Fixes:
Enhancements:
Documentation:
libmanifest field and its resolution behavior.Tests: