Skip to content

Conversation

@fank
Copy link

@fank fank commented Oct 6, 2025

This PR fixes several parsing issues that were causing errors when analyzing large codebases:

Issues Fixed

1. Function-scoped declarations

Problem: Function-local variables were being tracked as file-level declarations, causing "duplicate declaration" errors when the same variable name was used in multiple functions.

Solution: Modified the visitor to return nil after emitting FuncDecl nodes, preventing descent into function bodies. Import cycle detection only needs package-level declarations.

2. File emission order

Problem: All files were emitted when processing a Package node, but ImportSpec nodes were processed later during the AST walk. This caused all imports to be assigned to the last file in the package, leading to "duplicate import" errors.

Solution: Added package context tracking (currentPackage, currentDirName) to the visitor. File nodes are now emitted individually as they're encountered, with proper filename lookup from the package context.

3. Blank identifier

Problem: The blank identifier _ was being tracked like a regular declaration, but it's used for compile-time checks and can appear multiple times.

Solution: Skip tracking declarations named _ in the addGenDecl function.

4. External test packages

Problem: Packages ending in _test (external test packages) were being processed, but they can coexist with regular packages in the same directory and cannot be imported.

Solution: Added filtering in main.go to skip packages with names ending in _test.

5. Generic type receivers

Problem: Methods on generic types like *Foo[T] or Foo[T, U] were causing errors because the receiver name extraction didn't handle generic type syntax.

Solution: Added extractTypeName function that recursively extracts base type names from ast.IndexExpr and ast.IndexListExpr nodes. Updated receiver type handling to support generic types.

6. Error messages

Enhancement: Added file paths to all error messages for easier debugging.

Testing

These fixes were tested on a large codebase with:

  • Generic types with single and multiple type parameters
  • Extensive test coverage with external test packages
  • Multiple functions using the same local variable names
  • Compile-time interface checks using blank identifiers

The tool now successfully generates import cycle graphs for such codebases.

This commit addresses several parsing issues that caused errors when
analyzing large codebases:

1. Function-scoped declarations: The visitor now stops descending into
   function bodies after emitting FuncDecl nodes. This prevents
   function-local variables from being incorrectly tracked as file-level
   declarations, which was causing "duplicate declaration" errors.

2. File emission order: Fixed the AST visitor to properly track package
   context when emitting File nodes. Previously, all files were emitted
   at once when processing a Package node, causing ImportSpec nodes to
   be assigned to the wrong file. Now File nodes are emitted as they're
   encountered during the walk, using the stored package context to look
   up filenames.

3. Blank identifier: Added logic to skip tracking "_" declarations since
   the blank identifier can legitimately appear multiple times in a file
   (e.g., for compile-time interface checks).

4. External test packages: Added filtering to skip packages with names
   ending in "_test" since these are external test packages that cannot
   be imported and won't cause import cycles.

5. Generic type receivers: Added support for extracting receiver names
   from generic types like Foo[T] and *Foo[T]. The new extractTypeName
   function handles ast.IndexExpr and ast.IndexListExpr for generic
   types with one or multiple type parameters.

6. Error messages: Enhanced error messages to include file paths for
   easier debugging when duplicate declarations are detected.

These fixes allow the tool to successfully analyze complex codebases
with generics, extensive test suites, and varied declaration patterns.
@fank fank force-pushed the fix-ast-parsing-issues branch from 6a999bd to 272033e Compare October 6, 2025 15:26
@samlitowitz samlitowitz reopened this Nov 10, 2025
@samlitowitz
Copy link
Owner

  1. File emission order

Problem: All files were emitted when processing a Package node, but ImportSpec nodes were processed later during the AST walk. This caused all imports to be assigned to the last file in the package, leading to "duplicate import" errors.

Solution: Added package context tracking (currentPackage, currentDirName) to the visitor. File nodes are now emitted individually as they're encountered, with proper filename lookup from the package context.

Resolved by #14

@samlitowitz
Copy link
Owner

Some of these issues have been addressed and some are not issues.

@fank fank deleted the fix-ast-parsing-issues branch November 14, 2025 22:41
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