Skip to content

Conversation

@maks-ivanov
Copy link
Contributor

@maks-ivanov maks-ivanov commented Feb 10, 2025

User description

This commit introduces a more structured approach to representing file ranges by adding a nested Range type to FileRange. The changes include:

Make range optional for read source code
#64


CodeAnt-AI Description

  • Refactored the FileRange structure to include a nested Range type, enhancing the code's modularity and readability.
  • Updated multiple test files to accommodate the new file_range structure, ensuring compatibility and correctness.
  • Adjusted imports and references to use the new api_types::Range and lsp_types::Range where appropriate.

Changes walkthrough

Relevant files
Enhancement
python_tests.rs
Refactor Python tests to use nested `Range` type in `FileRange`

lsproxy/src/lsp/manager/language_tests/python_tests.rs

  • Introduced api_types::Range as a nested type within FileRange.
  • Updated test cases to use file_range instead of range.
  • Adjusted imports to include api_types.
  • +292/-232
    js_tests.rs
    Update JS tests to use consistent `Range` type                                 

    lsproxy/src/lsp/manager/language_tests/js_tests.rs

  • Changed Range to lsp_types::Range for consistency.
  • Updated test cases to use file_range instead of range.
  • Added import for Position and Range from api_types.
  • +45/-36 
    💡 Usage Guide

    Checking Your Pull Request

    Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

    Talking to CodeAnt AI

    Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

    @codeant-ai ask: Your question here
    

    This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

    Retrigger review

    Ask CodeAnt AI to review the PR again, by typing:

    @codeant-ai: review
    

    Check Your Repository Health

    To analyze the health of your code repository, visit our dashboard at app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

    This commit introduces a more structured approach to representing file ranges by adding a nested `Range` type to `FileRange`. The changes include:
    
     Make range optional for read source code
    @codeant-ai codeant-ai bot added the size:XXL This PR changes 1000+ lines, ignoring generated files label Feb 10, 2025
    @codeant-ai
    Copy link
    Contributor

    codeant-ai bot commented Feb 10, 2025

    Pull Request Feedback 🔍

    🔒 No security issues identified
    ⚡ Recommended areas for review

    Code Smell
    The repeated pattern of creating FileRange with nested Range and Position objects could be refactored to improve code readability and maintainability. Consider using a helper function or a builder pattern to reduce redundancy.

    Code Smell
    Similar to the C# tests, the repeated pattern of constructing FileRange objects with nested Range and Position could be refactored to enhance readability and reduce duplication.

    character: 0,
    },
    end: Position {
    line: definition.range.end.line as u32 + 3,
    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Suggestion: Consider using saturating_add instead of + 3 to prevent potential overflow when calculating the end line position. [possible issue]

    Suggested change
    line: definition.range.end.line as u32 + 3,
    line: definition.range.end.line.saturating_add(3),


    let expected = vec![
    Location {
    uri: Url::parse("file:///mnt/lsproxy_root/sample_project/java/AStar.java").unwrap(),
    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Suggestion: Validate that the Url::parse method is used correctly to handle potential errors gracefully. [best practice]

    Suggested change
    uri: Url::parse("file:///mnt/lsproxy_root/sample_project/java/AStar.java").unwrap(),
    uri: Url::parse("file:///mnt/lsproxy_root/sample_project/java/AStar.java").expect("Failed to parse URL"),

    …ures
    
    This commit updates the OpenAPI schema to consistently use the new `Range` and `FileRange` types across multiple schemas. Key changes include:
    - Renaming `range` to `file_range` in symbol and definition schemas
    - Adding a new `ReadSourceCodeRequest` schema with optional range
    - Modifying `FileRange` to use nested `Range` type
    - Adjusting request and response schemas to reflect these structural changes
    @maks-ivanov maks-ivanov reopened this Feb 10, 2025
    @maks-ivanov
    Copy link
    Contributor Author

    @maks-ivanov maks-ivanov changed the title refactor: Restructure FileRange to use nested Range type Make Range optional in read source code endpoint Feb 10, 2025
    Copy link
    Contributor

    @robmck1995 robmck1995 left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    LGTM

    @maks-ivanov maks-ivanov merged commit 586a58d into main Feb 10, 2025
    3 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    size:XXL This PR changes 1000+ lines, ignoring generated files

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    3 participants