Skip to content
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

Fix linter to allow subscripted annotations (e.g. list[int], dict[str, int]) #114

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Endogen
Copy link
Contributor

@Endogen Endogen commented Jan 14, 2025

Summary

This PR fixes an issue where our linter rejects subscripted annotations (e.g. list[int]) with an S16- Illegal argument annotation used error, even though the base type (list, dict, etc.) is in our allowed list of annotation types.

Problem

  • Our current linter checks argument annotations against a set of ALLOWED_ANNOTATION_TYPES. However, we only include strings like "list", "dict", "int", etc.
  • When an annotation is subscripted (e.g., list[int], dict[str, int]), we store it as a string like "list[int]". Since "list[int]" is not strictly in ALLOWED_ANNOTATION_TYPES, it triggers the S16 violation.

Example code that triggers the error:

@export
def func(x: list[int]):
    pass

Result: The linter reports Line X : S16- Illegal argument annotation used : list[int].

Proposed Changes

  1. Safely parse subscripted annotations (list[int], dict[str,int], etc.) to recognize the base type (list, dict) during linting.
  2. In the annotation_types method, if the string contains [, extract the portion before it (e.g., "list" from "list[int]" or "list[list[int]]"). If that base is in ALLOWED_ANNOTATION_TYPES, allow the annotation.

Minimal Example of the Annotation Check

def annotation_types(self, t, lnum):
    if t is None:
        str_msg = "Line {}".format(lnum) + " : " + VIOLATION_TRIGGERS[16]
        self._violations.append(str_msg)
        self._is_success = False

    else:
        # If there's a subscript portion (e.g. "list[int]" or "dict[str,int]"),
        # extract everything before the "[" and use that as the base type.
        if "[" in t:
            base_type = t.split("[", 1)[0]
            if base_type in ALLOWED_ANNOTATION_TYPES:
                return  # If the base type is allowed, consider it valid and stop checking.

        # If it's not subscripted, or if the base doesn't match,
        # fall back to the original check.
        if t not in ALLOWED_ANNOTATION_TYPES:
            str_msg = "Line {}".format(lnum) + " : " + VIOLATION_TRIGGERS[15] + " : {}".format(t)
            self._violations.append(str_msg)
            self._is_success = False

With this tweak, the linter will permit subscripted forms of otherwise allowed types like list or dict.

Testing

  • Added/Modified a test case with a function using subscripted annotations:
    @export
    def func(x: list[int], y: dict[str, int]):
        return
  • Verified the linter now passes without producing an S16 violation.
  • Existing tests for other illegal annotations continue to pass.

Rationale and Benefits

  • Enables the use of modern Python typing features (list[int], dict[str, T], etc.) without artificially blocking them in the linter.
  • Keeps the base rule that only certain “core types” (list, dict, etc.) are allowed, but extends it gracefully to subscripted types.

Additional Notes

  • This PR does not add any new annotation types to ALLOWED_ANNOTATION_TYPES. It merely makes the existing ones more flexible for subscript usage.
  • The helper function (_get_annotation_name or any similar logic) remains as is—this change focuses on the final check to see if a subscript is allowed.

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have performed a self-review of my own code
  • I have tested this change in my development environment.
  • I have added tests to prove that this change works
  • All existing tests pass after this change
  • I have added / updated documentation related to this change

@Endogen Endogen added the enhancement New feature or request label Jan 14, 2025
@Endogen Endogen self-assigned this Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant