fix import parser bug

[tastelikefeet]

(cherry picked from commit fa0f33866054a7018885cf8f0d7026f5417788ed)

Change Summary

Related issue number

Checklist

• The pull request title is a good summary of the changes - it will be used in the changelog
• Unit tests for the changes exist
• Run pre-commit install and pre-commit run --all-files before git commit, and passed lint check.
• Documentation reflects the changes where applicable

[gemini-code-assist]

Summary of Changes

Hello @tastelikefeet, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses and resolves a significant bug in the import parser that caused incorrect path resolution, particularly in complex file system environments involving relative output directories or macOS symlinks. The core fix involves introducing a more robust path resolution utility, which is thoroughly validated by new, comprehensive test cases. Additionally, the PR includes updates to code documentation guidelines, promoting better clarity and information capture within code comments.

Highlights

• Import Parser Bug Fix: A critical bug in the import parser, specifically affecting path resolution with relative output directories and macOS symlinks, has been fixed. This ensures accurate import path identification across various file system configurations.
• Robust Path Resolution: A new helper function, safe_relpath, was introduced in parser_utils.py. This function canonicalizes paths using os.path.realpath and os.path.abspath to correctly handle mixed absolute/relative paths and resolve symbolic links, preventing incorrect path generation.
• Expanded Test Coverage: New regression tests have been added to test_parse_imports.py for both Python and JavaScript/TypeScript. These tests specifically validate the corrected behavior of the import parser in scenarios that previously led to path resolution errors, such as those involving relative output directories and symlinks.
• Code Documentation Guidelines: Updates were made to code_condenser.py and coding.yaml to emphasize the importance of detailed comments and docstrings for public/export methods and classes. These guidelines now explicitly require descriptions of input and output structures, aiming to improve code clarity and analysis.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request addresses a bug in the import parser concerning path resolution, particularly when dealing with relative output directories and symlinks. The introduction of the safe_relpath function for Python path resolution is a solid fix, and it's well-supported by new regression tests for both Python and JavaScript/TypeScript.

My main feedback is that the fix is incomplete. The robust path resolution logic using os.path.realpath has been applied to the Python parser, but a similar function in the JavaScript/TypeScript parser was not updated. This leads to code duplication and leaves the symlink-related bug unresolved for JS/TS files. I've left a specific comment with a suggestion to refactor this into a shared method to ensure consistency and fully resolve the issue.

[gemini-code-assist]

This new helper function safe_relpath is a great addition and correctly solves the path resolution issue, including symlink problems on macOS by using os.path.realpath.

However, I noticed two issues:

1. Incomplete Fix: A similar, but less robust, inner function to_relative exists inside _resolve_js_path (lines 562-579) which was not updated to use os.path.realpath. This means the symlink resolution bug is fixed for Python imports but still exists for JavaScript/TypeScript imports. The new JS test (test_relative_output_dir_js_no_excessive_parent_dirs) might be passing coincidentally in an environment without symlinked temp directories.
2. Code Duplication: Defining safe_relpath as an inner function prevents its reuse.

To improve maintainability and ensure the bug is fixed consistently across all parsers, I recommend refactoring this. You could move safe_relpath to BaseImportParser as a private helper method (e.g., _safe_relpath) and then call it from both PythonImportParser and JavaScriptImportParser, removing the duplicated logic from _resolve_js_path.