-
Notifications
You must be signed in to change notification settings - Fork 3
Static type checks #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: varun-r-mallya <varunrmallya@gmail.com>
please check for breakages @r41k0u on your side |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds static type checking support to the codebase by enabling mypy type checking and adding type annotations to improve code quality and maintainability.
- Enables mypy type checking in pre-commit configuration
- Adds type annotations to class attributes and function parameters/return types
- Excludes tests and examples from type checking and formatting
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
pythonbpf/maps/maps_utils.py | Adds type annotations for the _processors class attribute |
pythonbpf/functions_pass.py | Adds type annotations for global variable and function return type |
pythonbpf/codegen.py | Wraps subprocess call result in bool() for type consistency |
.pre-commit-config.yaml | Enables mypy type checking and updates exclusion patterns |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
from .expr_pass import eval_expr, handle_expr | ||
|
||
local_var_metadata = {} | ||
local_var_metadata: dict[str | Any, Any] = {} |
Copilot
AI
Oct 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type annotation dict[str | Any, Any]
is overly permissive. Consider using a more specific type for the key (e.g., just str
if all keys are strings) or define a TypedDict if the structure is predictable.
local_var_metadata: dict[str | Any, Any] = {} | |
local_var_metadata: dict[str, str] = {} |
Copilot uses AI. Check for mistakes.
pythonbpf/functions_pass.py
Outdated
|
||
|
||
def infer_return_type(func_node: ast.FunctionDef): | ||
def infer_return_type(func_node: ast.FunctionDef) -> str | None | Any: |
Copilot
AI
Oct 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type str | None | Any
defeats the purpose of type checking. Consider using a more specific union type or Optional[str] if the function can return a string or None.
Copilot uses AI. Check for mistakes.
success = compile_to_ir(str(caller_file), str(ll_file)) and success | ||
|
||
success = ( | ||
success = bool( |
Copilot
AI
Oct 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The explicit bool()
wrapper is unnecessary. The subprocess.run() result in a boolean context already evaluates correctly for the and
operation on line 142.
Copilot uses AI. Check for mistakes.
Signed-off-by: varun-r-mallya <varunrmallya@gmail.com>
Adds static type checking to code. Does not check examples and tests.