Skip to content

Commit

Permalink
fix(export-map): incorrect internal namespaces info (#64)
Browse files Browse the repository at this point in the history
  • Loading branch information
JounQin authored Mar 24, 2024
1 parent 2eca701 commit b858aee
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 71 deletions.
5 changes: 5 additions & 0 deletions .changeset/stale-sheep-yawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-import-x": patch
---

fix(export-map): incorrect internal `namespaces` info
11 changes: 7 additions & 4 deletions src/rules/export.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import type { TSESTree } from '@typescript-eslint/utils'

import { ExportMap, recursivePatternCapture, createRule } from '../utils'
import {
ExportMap,
recursivePatternCapture,
createRule,
getValue,
} from '../utils'

/*
Notes on TypeScript namespaces aka TSModuleDeclaration:
Expand Down Expand Up @@ -181,9 +186,7 @@ export = createRule<[], MessageId>({

ExportSpecifier(node) {
addNamed(
node.exported.name ||
// @ts-expect-error - legacy parser type
node.exported.value,
getValue(node.exported),
node.exported,
getParent(node.parent!),
)
Expand Down
5 changes: 2 additions & 3 deletions src/rules/namespace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
ExportMap,
createRule,
declaredScope,
getValue,
} from '../utils'

type MessageId =
Expand Down Expand Up @@ -62,9 +63,7 @@ function processBodyStatement(
case 'ImportSpecifier': {
const meta = imports.get(
'imported' in specifier && specifier.imported
? specifier.imported.name ||
// @ts-expect-error - legacy parser node
specifier.imported.value
? getValue(specifier.imported)
: // default to 'default' for default
'default',
)
Expand Down
2 changes: 1 addition & 1 deletion src/rules/no-default-export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export = createRule({
'default',
)) {
const { loc } = getSourceCode(context).getFirstTokens(node)[1] || {}
// @ts-expect-error - legacy parser type
// @ts-expect-error - experimental parser type
if (specifier.type === 'ExportDefaultSpecifier') {
context.report({
node,
Expand Down
6 changes: 2 additions & 4 deletions src/rules/no-named-default.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createRule } from '../utils'
import { createRule, getValue } from '../utils'

type MessageId = 'default'

Expand Down Expand Up @@ -31,9 +31,7 @@ export = createRule<[], MessageId>({

if (
im.type === 'ImportSpecifier' &&
(im.imported.name ||
// @ts-expect-error - legacy parser type
im.imported.value) === 'default'
getValue(im.imported) === 'default'
) {
context.report({
node: im.local,
Expand Down
26 changes: 5 additions & 21 deletions src/rules/no-unused-modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
getFileExtensions,
readPkgUp,
visit,
getValue,
} from '../utils'

function listFilesToProcess(src: string[], extensions: FileExtension[]) {
Expand Down Expand Up @@ -633,11 +634,7 @@ export = createRule<Options[], MessageId>({
if (s.specifiers.length > 0) {
for (const specifier of s.specifiers) {
if (specifier.exported) {
newExportIdentifiers.add(
specifier.exported.name ||
// @ts-expect-error - legacy parser type
specifier.exported.value,
)
newExportIdentifiers.add(getValue(specifier.exported))
}
}
}
Expand Down Expand Up @@ -753,10 +750,7 @@ export = createRule<Options[], MessageId>({
context,
)
for (const specifier of astNode.specifiers) {
const name =
specifier.local.name ||
// @ts-expect-error - legacy parser type
specifier.local.value
const name = getValue(specifier.local)
if (name === DEFAULT) {
newDefaultImports.add(resolvedPath!)
} else {
Expand Down Expand Up @@ -800,12 +794,7 @@ export = createRule<Options[], MessageId>({
specifier.type !== AST_NODE_TYPES.ImportNamespaceSpecifier,
)) {
if ('imported' in specifier) {
newImports.set(
specifier.imported.name ||
// @ts-expect-error - legacy parser type
specifier.imported.value,
resolvedPath,
)
newImports.set(getValue(specifier.imported), resolvedPath)
}
}
}
Expand Down Expand Up @@ -1004,12 +993,7 @@ export = createRule<Options[], MessageId>({
},
ExportNamedDeclaration(node) {
for (const specifier of node.specifiers) {
checkUsage(
specifier,
specifier.exported.name ||
// @ts-expect-error - legacy parser type
specifier.exported.value,
)
checkUsage(specifier, getValue(specifier.exported))
}
forEachDeclarationIdentifier(node.declaration, name => {
checkUsage(node, name)
Expand Down
40 changes: 15 additions & 25 deletions src/utils/export-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import type {
RuleContext,
} from '../types'

import { getValue } from './get-value'
import { hashObject } from './hash'
import { hasValidExtension, ignore } from './ignore'
import { parse } from './parse'
Expand Down Expand Up @@ -212,7 +213,7 @@ export class ExportMap {
})
}

const namespaces = new Map()
const namespaces = new Map</* identifier */ string, /* source */ string>()

function remotePath(value: string) {
return relative(value, filepath, context.settings)
Expand All @@ -226,25 +227,18 @@ export class ExportMap {
return ExportMap.for(childContext(rp, context))
}

function getNamespace(identifier: TSESTree.Identifier | string) {
const namespace =
typeof identifier === 'string' ? identifier : identifier.name
function getNamespace(namespace: string) {
if (!namespaces.has(namespace)) {
return
}

return function () {
return resolveImport(namespaces.get(namespace))
return resolveImport(namespaces.get(namespace)!)
}
}

function addNamespace(
object: object,
identifier: TSESTree.Identifier | string,
) {
const namespace =
typeof identifier === 'string' ? identifier : identifier.name
const nsfn = getNamespace(namespace)
function addNamespace(object: object, identifier: TSESTree.Identifier) {
const nsfn = getNamespace(getValue(identifier))
if (nsfn) {
Object.defineProperty(object, 'namespace', { get: nsfn })
}
Expand Down Expand Up @@ -289,19 +283,16 @@ export class ExportMap {
}
case 'ExportAllDeclaration': {
m.namespace.set(
s.exported!.name ||
// @ts-expect-error - legacy parser type
s.exported!.value,
addNamespace(exportMeta, s.source.value),
getValue(s.exported!),
addNamespace(exportMeta, s.exported!),
)

return
}
case 'ExportSpecifier': {
if (!('source' in n && n.source)) {
m.namespace.set(
s.exported.name ||
// @ts-expect-error - legacy parser type
s.exported.value,
getValue(s.exported),
addNamespace(exportMeta, s.local),
)
return
Expand Down Expand Up @@ -342,11 +333,7 @@ export class ExportMap {
const importedSpecifiers = new Set<string>()
for (const specifier of n.specifiers) {
if (specifier.type === 'ImportSpecifier') {
importedSpecifiers.add(
specifier.imported.name ||
// @ts-expect-error - legacy parser type
specifier.imported.value,
)
importedSpecifiers.add(getValue(specifier.imported))
} else if (supportedImportTypes.has(specifier.type)) {
importedSpecifiers.add(specifier.type)
}
Expand Down Expand Up @@ -460,6 +447,7 @@ export class ExportMap {
m.dependencies.add(getter)
}
if (n.exported) {
namespaces.set(n.exported.name, n.source.value)
processSpecifier(n, n.exported, m)
}
continue
Expand Down Expand Up @@ -520,7 +508,9 @@ export class ExportMap {
}
}

for (const s of n.specifiers) processSpecifier(s, n, m)
for (const s of n.specifiers) {
processSpecifier(s, n, m)
}
}

const exports = ['TSExportAssignment']
Expand Down
17 changes: 17 additions & 0 deletions src/utils/get-value.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { TSESTree } from '@typescript-eslint/utils'

export const getValue = (
node: TSESTree.Identifier | TSESTree.StringLiteral,
) => {
switch (node.type) {
case TSESTree.AST_NODE_TYPES.Identifier: {
return node.name
}
case TSESTree.AST_NODE_TYPES.Literal: {
return node.value
}
default: {
throw new Error(`Unsupported node type: ${(node as TSESTree.Node).type}`)
}
}
}
1 change: 1 addition & 0 deletions src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export * from './docs-url'
export * from './export-map'
export * from './get-ancestors'
export * from './get-scope'
export * from './get-value'
export * from './hash'
export * from './ignore'
export * from './import-declaration'
Expand Down
40 changes: 27 additions & 13 deletions test/utils/export-map.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,14 @@ describe('ExportMap', () => {
.readFileSync(path, { encoding: 'utf8' })
.replaceAll(/\r?\n/g, lineEnding)

const imports = ExportMap.parse(path, contents, parseContext)!
let imports: ExportMap

// sanity checks
expect(imports.errors).toHaveLength(0)
beforeAll(() => {
imports = ExportMap.parse(path, contents, parseContext)!

// sanity checks
expect(imports.errors).toHaveLength(0)
})

it('works with named imports.', () => {
expect(imports.has('fn')).toBe(true)
Expand Down Expand Up @@ -184,10 +188,15 @@ describe('ExportMap', () => {
describe('full module', () => {
const path = testFilePath('deprecated-file.js')
const contents = fs.readFileSync(path, { encoding: 'utf8' })
const imports = ExportMap.parse(path, contents, parseContext)!

// sanity checks
expect(imports.errors).toHaveLength(0)
let imports: ExportMap

beforeAll(() => {
imports = ExportMap.parse(path, contents, parseContext)!

// sanity checks
expect(imports.errors).toHaveLength(0)
})

it('has JSDoc metadata', () => {
expect(imports.doc).toBeDefined()
Expand Down Expand Up @@ -281,8 +290,7 @@ describe('ExportMap', () => {
expect(def.get('default')!.namespace!.has('c')).toBe(true)
})

// FIXME: check and enable
it.skip('works with @babel/eslint-parser & ES7 namespace exports', function () {
it('works with @babel/eslint-parser & ES7 namespace exports', function () {
const path = testFilePath('deep-es7/a.js')
const contents = fs.readFileSync(path, { encoding: 'utf8' })
const a = ExportMap.parse(path, contents, babelContext)!
Expand Down Expand Up @@ -376,10 +384,8 @@ describe('ExportMap', () => {
settings: { 'import-x/extensions': ['.js'] as const },
}

const imports = ExportMap.get('./typescript.ts', context)

it('returns nothing for a TypeScript file', () => {
expect(imports).toBeFalsy()
expect(ExportMap.get('./typescript.ts', context)).toBeFalsy()
})
})

Expand All @@ -395,9 +401,17 @@ describe('ExportMap', () => {

jest.setTimeout(20e3) // takes a long time :shrug:

const spied = jest.spyOn(getTsconfig, 'getTsconfig').mockClear()
const spied = jest.spyOn(getTsconfig, 'getTsconfig')

let imports: ExportMap

const imports = ExportMap.get('./typescript.ts', context)!
beforeAll(() => {
imports = ExportMap.get('./typescript.ts', context)!
})

beforeEach(() => {
spied.mockClear()
})

afterAll(() => {
spied.mockRestore()
Expand Down

0 comments on commit b858aee

Please sign in to comment.