Repository navigation
Reintegrate package library entry points into main - #15
Merged
Merged
Conversation
Contributor
Reviewer's GuideReintegrates explicit package library entry points across both runtime source copies, with manifest/path validation, strict resolution semantics, backward-compatible fallbacks, documentation, and an end-to-end package test. Sequence diagram for package entry point resolutionsequenceDiagram
participant Importer
participant Runtime
participant Package
participant Filesystem
Importer->>Runtime: sys_resolve_module(path)
Runtime->>Package: pkg_declared_lib(package_dir, declared)
Package->>Filesystem: Read flint.toml
Filesystem-->>Package: [package] lib value
alt lib declared
Package->>Filesystem: Validate and locate declared .fl file
Filesystem-->>Runtime: Entry point or missing result
Runtime-->>Importer: Module or import error
else lib not declared
Runtime->>Filesystem: Check main.fl, same-named .fl, flat .fl
Filesystem-->>Runtime: First matching legacy module
Runtime-->>Importer: Resolved module
end
Flow diagram for explicit package library resolutionflowchart TD
Import["Bare package import"] --> PackageDir["Locate installed package directory"]
PackageDir --> Manifest["Read flint.toml [package] lib"]
Manifest --> Declared{Declared lib?}
Declared -->|Yes| Validate["Validate relative .fl path"]
Validate --> Entry["Resolve declared entry point"]
Declared -->|No| Legacy["Try main.fl, same-named .fl, then flat .fl"]
Entry --> Result["Return module or import error"]
Legacy --> Result
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
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 |
![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 4 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-573" />
<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;
+ char *main_file = malloc(need);
</code_context>
<issue_to_address>
**Invalid package entry loads another module**
When a package declares a missing or invalid `lib` target and another module with the same name is reachable, `sys_resolve_module` continues to standard-library or relative resolution, and `import_file` can try an importer sibling, so the import loads a different module instead of reporting the invalid package entry point.
When a declared `lib` target cannot be resolved, stop module resolution and report that package entry point as invalid.
Also at `rust/flintlang/c-src/src/runtime/sys.c:571-573`.
</issue_to_address>
### Comment 2
<location path="src/runtime/sys.c" line_range="487-488" />
<code_context>
+ *declared = true;
+ if (*p++ != '"')
+ break;
+ char *end = strchr(p, '"');
+ if (end == NULL)
+ break;
+ char *tail = end + 1;
</code_context>
<issue_to_address>
**Escaped entry paths fail to resolve**
When a declared `lib` path contains a TOML escape, including an escaped quote or Unicode escape, `pkg_declared_lib` reads the raw TOML value, so it treats an escaped quote as the closing delimiter or rejects the backslash instead of resolving the decoded path. The package import fails even though the decoded target is present.
Parse and decode the TOML string value before validating and resolving the declared path.
Also at `rust/flintlang/c-src/src/runtime/sys.c:487-500`, `src/runtime/sys.c:498-500`.
</issue_to_address>
### Comment 3
<location path="src/runtime/sys.c" line_range="507-510" />
<code_context>
+ if (*q == '/' || *q == '\0') {
+ size_t part_len = (size_t)(q - part);
+ if (part_len == 0 ||
+ (part_len == 1 && part[0] == '.') ||
+ (part_len == 2 && part[0] == '.' &&
+ part[1] == '.')) {
+ unsafe = true;
+ break;
+ }
</code_context>
<issue_to_address>
**Dot-prefixed paths fail to resolve**
When a package declares an existing `lib` target using a `.` path component, such as `./src/api.fl`, `pkg_declared_lib` marks the `.` component unsafe and returns no target, so `sys_resolve_module` skips legacy layouts and the import fails despite the target file existing.
Allow safe `.` path components when validating an in-package declared path.
Also at `rust/flintlang/c-src/src/runtime/sys.c:507-510`.
</issue_to_address>
### Comment 4
<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>
**Declared packages are missing import hints**
When a package is available through its declared `lib` path, has no `main.fl`, and code references its name without importing it, `sys_module_file_exists` checks only `<name>/main.fl`, so `unimported_module` omits the “forgot to import” hint for the declared package.
Have `sys_module_file_exists` recognize packages resolved through their declared `lib` path.
Also at `rust/flintlang/c-src/src/runtime/sys.c:571-572`.
</issue_to_address>Sourcery assessment
Approval pending. 3 findings to address first.
Blocking findings: src/runtime/sys.c:573, src/runtime/sys.c:488, src/runtime/sys.c:510
Comment on lines
+571
to
+573
| found = pkg_declared_lib( | ||
| dir_main, &has_declared_lib); | ||
| if (!has_declared_lib) { |
Contributor
There was a problem hiding this comment.
🟡 Medium · Invalid package entry loads another module
When a package declares a missing or invalid lib target and another module with the same name is reachable, sys_resolve_module continues to standard-library or relative resolution, and import_file can try an importer sibling, so the import loads a different module instead of reporting the invalid package entry point.
When a declared lib target cannot be resolved, stop module resolution and report that package entry point as invalid.
Also at rust/flintlang/c-src/src/runtime/sys.c:571-573.
Prompt for AI agents
In `src/runtime/sys.c` at lines 571-573:
**Invalid package entry loads another module**
When a package declares a missing or invalid `lib` target and another module with the same name is reachable, `sys_resolve_module` continues to standard-library or relative resolution, and `import_file` can try an importer sibling, so the import loads a different module instead of reporting the invalid package entry point.
When a declared `lib` target cannot be resolved, stop module resolution and report that package entry point as invalid.
Also at `rust/flintlang/c-src/src/runtime/sys.c:571-573`.
Comment on lines
+487
to
+488
| char *end = strchr(p, '"'); | ||
| if (end == NULL) |
Contributor
There was a problem hiding this comment.
🟡 Medium · Escaped entry paths fail to resolve
When a declared lib path contains a TOML escape, including an escaped quote or Unicode escape, pkg_declared_lib reads the raw TOML value, so it treats an escaped quote as the closing delimiter or rejects the backslash instead of resolving the decoded path. The package import fails even though the decoded target is present.
Parse and decode the TOML string value before validating and resolving the declared path.
Also at rust/flintlang/c-src/src/runtime/sys.c:487-500, src/runtime/sys.c:498-500.
Prompt for AI agents
In `src/runtime/sys.c` at lines 487-488:
**Escaped entry paths fail to resolve**
When a declared `lib` path contains a TOML escape, including an escaped quote or Unicode escape, `pkg_declared_lib` reads the raw TOML value, so it treats an escaped quote as the closing delimiter or rejects the backslash instead of resolving the decoded path. The package import fails even though the decoded target is present.
Parse and decode the TOML string value before validating and resolving the declared path.
Also at `rust/flintlang/c-src/src/runtime/sys.c:487-500`, `src/runtime/sys.c:498-500`.
Comment on lines
+507
to
+510
| (part_len == 1 && part[0] == '.') || | ||
| (part_len == 2 && part[0] == '.' && | ||
| part[1] == '.')) { | ||
| unsafe = true; |
Contributor
There was a problem hiding this comment.
🟡 Medium · Dot-prefixed paths fail to resolve
When a package declares an existing lib target using a . path component, such as ./src/api.fl, pkg_declared_lib marks the . component unsafe and returns no target, so sys_resolve_module skips legacy layouts and the import fails despite the target file existing.
Allow safe . path components when validating an in-package declared path.
Also at rust/flintlang/c-src/src/runtime/sys.c:507-510.
Prompt for AI agents
In `src/runtime/sys.c` at lines 507-510:
**Dot-prefixed paths fail to resolve**
When a package declares an existing `lib` target using a `.` path component, such as `./src/api.fl`, `pkg_declared_lib` marks the `.` component unsafe and returns no target, so `sys_resolve_module` skips legacy layouts and the import fails despite the target file existing.
Allow safe `.` path components when validating an in-package declared path.
Also at `rust/flintlang/c-src/src/runtime/sys.c:507-510`.
Comment on lines
+571
to
+572
| found = pkg_declared_lib( | ||
| dir_main, &has_declared_lib); |
Contributor
There was a problem hiding this comment.
⚪ Low · Declared packages are missing import hints
When a package is available through its declared lib path, has no main.fl, and code references its name without importing it, sys_module_file_exists checks only <name>/main.fl, so unimported_module omits the “forgot to import” hint for the declared package.
Have sys_module_file_exists recognize packages resolved through their declared lib path.
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:
**Declared packages are missing import hints**
When a package is available through its declared `lib` path, has no `main.fl`, and code references its name without importing it, `sys_module_file_exists` checks only `<name>/main.fl`, so `unimported_module` omits the “forgot to import” hint for the declared package.
Have `sys_module_file_exists` recognize packages resolved through their declared `lib` path.
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.
Recovery PR: the original #14 is marked merged, but a fresh comparison against the current main ref still shows the four package-lib commits absent. Reapply the already-green change against main and require current CI before merging.
Summary by Sourcery
Reintegrate manifest-defined package library entry points into package installation and module resolution.
New Features:
.flpublic entry point through the manifest's[package] libfield.Bug Fixes:
Documentation:
Tests: