Skip to content

fix(import-detect): drop commented-out and relative imports - #85

Merged
jpbelmo merged 4 commits into
Redential:mainfrom
eeshsaxena:fix/import-detect-comments-relative
Aug 18, 2026
Merged

fix(import-detect): drop commented-out and relative imports#85
jpbelmo merged 4 commits into
Redential:mainfrom
eeshsaxena:fix/import-detect-comments-relative

Conversation

@eeshsaxena

Copy link
Copy Markdown
Contributor

Three false positives in extractImportedPackages (the added-line import scanner feeding skill detection). Each makes it attribute a dependency the code doesn't actually import, which the module's "bounded false positives" contract is meant to prevent.

1. Commented-out imports inside a Go import (...) block

The single-line Go form already drops // import "..." via isRealStatement, but the block form scans every quoted path in the block body, comments included:

import (
	"fmt"
	// "github.com/spf13/cobra"  // temporarily disabled
	"github.com/gin-gonic/gin"
)

Before: ["fmt", "github.com/spf13/cobra", "github.com/gin-gonic/gin"] — the commented-out cobra is credited.
After: ["fmt", "github.com/gin-gonic/gin"].

The block body now skips isCommentLine lines, matching the single-line path.

2. JS relative/absolute specifiers leak . / ..

normalizeJs("./util") returns "." (and "../lib/x" returns ".."), so every relative import adds a bogus token:

import { a } from "./util";
import b from "../lib/x";
import c from "react";

Before: [".", "..", "react"] · After: ["react"]

3. Python relative from-imports leak ""

from . import x / from .models import Y split to an empty first segment, pushed unguarded (the sibling import branch already guards with if (name)):

from . import helpers
from .models import User
import django

Before: ["", "", "django"] · After: ["django"]

#2 and #3 are the same "a relative module is not a package" case that Ruby's require_relative and Rust's crate/self/super are already excluded for; JS and Python just weren't.

Tests

Adds three regression tests to test/import-detect.test.ts (one per case). They fail on main and pass with this change; the full import-detect suite stays green (90 passed).

Three false positives in the added-line import scanner:

- Go import blocks matched quoted paths on every line, including
  `//`-commented ones, so a commented-out `// "github.com/foo/bar"` inside
  a block was attributed as a real dependency. The single-line form already
  rejects comments via isRealStatement; the block body now skips
  isCommentLine lines too.
- JS relative/absolute specifiers ("./util", "../lib/x", "/abs") were
  normalized to a bare "." or ".." and leaked into the candidate list.
- Python relative from-imports ("from . import x", "from .m import Y")
  produced an empty "" candidate.

Both now skip local specifiers, the same intent Ruby's require_relative and
Rust's crate/self/super exclusions already encode. Adds regression tests for
all three.
@eeshsaxena

Copy link
Copy Markdown
Contributor Author

Hi! Gentle nudge on this one whenever you have some bandwidth. It's a small, self-contained fix (fix(import-detect): drop commented-out and relative imports), and it's currently mergeable with no conflicts. No urgency at all, and I'm happy to make any changes you'd like. Thanks for maintaining redential-cli!

@jpbelmo
jpbelmo merged commit 8a718a8 into Redential:main Aug 18, 2026
10 checks passed
@jpbelmo

jpbelmo commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Merged, and sorry for the three quiet days — the binaries release ate the repo's attention, which isn't how we treat a fix this clean. All three bugs reproduced exactly as claimed, and the Go one was a genuine false-attribution vector: exactly the class of thing #27 exists to hunt. I added the changelog line at merge so you didn't need another round. The trailing-comment-on-kept-Go-lines sibling you'll probably enjoy: it's yours if you want it, follow-up issue coming.

jpbelmo added a commit that referenced this pull request Aug 18, 2026
Three features, three contributors: corporate proxy support with honest
network errors (#83), truthful import extraction (#85), and npm
release-date anachronism checks (#81).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015cG33p5HAAqi81QNiSo4Yx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants