Skip to content

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

Open
eeshsaxena wants to merge 1 commit into
Redential:mainfrom
eeshsaxena:fix/import-detect-comments-relative
Open

fix(import-detect): drop commented-out and relative imports#85
eeshsaxena wants to merge 1 commit 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!

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.

1 participant