Auto-import fails when a identifier is the same as a name to be imported

[GoodbyeNJN]

Environment

unimport version: v3.3.0
node.js version: v18.16.0

Reproduction

import { createUnimport } from 'unimport'

const code = `
{
  const fn = () => {};
};

fn();
`

const { injectImports } = createUnimport({
  imports: [{ name: 'fn', from: 'module' }]
})

const { imports } = await injectImports(code)
console.log(imports)

Describe the bug

unimport/src/context.ts

Lines 247 to 256 in 28cf011

	// Remove those already defined
	for (const regex of excludeRE) {
	for (const match of strippedCode.matchAll(regex)) {
	const segments = [...match[1]?.split(separatorRE) || [], ...match[2]?.split(separatorRE) || []]
	for (const segment of segments) {
	const identifier = segment.replace(importAsRE, '').trim()
	occurrenceMap.delete(identifier)
	}
	}
	}

Before Remove those already defined, the value of occurrenceMap is { size:1, [{ "fn" => 30 }] }, where fn refers to the function which is called outside of the block scope.

While run into the for loop, strippedCode will match regexp /\b(?:const|let|var)\s+?(\[.*?\]|\{.*?\}|.+?)\s*?[=;\n]/gs, and matching result is [ "const fn =", "fn" ], where fn refers to the function defined inside the block scope. So occurrenceMap.delete(identifier) will be executed, occurrenceMap will be cleared, and finally no import statement will be prepended to the code.

Seems in this situation, regexp matching is not enough, and lexical analysis may be needed?

Additional context

No response

Logs

No response

[lehni]

We've also encountered this… I wonder if the only way to make unimport work reliably would be to stop using regexps and use AST parsing and processing instead?

Related: #250 (comment)

@antfu what do you think? We keep seeing auto-import issues every now and then in our large code-base, and I can't always figure out why, or find the time to create a bug report. Most of the time, I do changes where I then rename a variable (as here), or change the way the auto-imported function is used. It's not exactly inspiring confidence…

[antfu]

Yeah, this is indeed sometimes impossible to fix with regex. I think that makes sense to introduce an optional mode to do full parsing.

[antfu]

Created #304 to track on that.

[antfu]

#304 is implemented, try with parser: 'acorn'