Skip to content

Comments

Python: Add metrics query for type annotations#18448

Merged
tausbn merged 1 commit intomainfrom
tausbn/python-add-type-annotation-metrics-query
Mar 6, 2025
Merged

Python: Add metrics query for type annotations#18448
tausbn merged 1 commit intomainfrom
tausbn/python-add-type-annotation-metrics-query

Conversation

@tausbn
Copy link
Contributor

@tausbn tausbn commented Jan 8, 2025

Adds a query that counts the number of type annotations of various kinds. Intended to be used with something like MRVA to inform our modelling decisions.

Currently the query counts the following "interesting" types in addition to the total number of types:

  • Built-in types (which are less likely to be interesting from a modelling perspective)
  • Forward declarations (i.e. annotations inside strings) which will require a fair bit of QL machinery to interpret.
  • Simple types (stuff like foo or foo.bar.baz)
  • Optional types (stuff like Optional[foo] which from a modelling perspective should likely be treated the same as foo)
  • Complex types (anything that contains more complex type constructions such as instantiations of generic types)

Pull Request checklist

All query authors

Internal query authors only

  • Autofixes generated based on these changes are valid, only needed if this PR makes significant changes to .ql, .qll, or .qhelp files. See the documentation (internal access required).
  • Changes are validated at scale (internal access required).
  • Adding a new query? Consider also adding the query to autofix.

Adds a query that counts the number of type annotations of various
kinds. Intended to be used with something like MRVA to inform our
modelling decisions.

Currently the query counts the following "interesting" types in addition
to the total number of types:
- Built-in types (which are less likely to be interesting from a
modelling perspective)
- Forward declarations (i.e. annotations inside strings) which will
require a fair bit of QL machinery to interpret.
- Simple types (stuff like `foo` or `foo.bar.baz`)
- Optional types (stuff like `Optional[foo]` which from a modelling
perspective should likely be treated the same as `foo`)
- Complex types (anything that contains more complex type constructions
such as instantiations of generic types)
@tausbn tausbn added the no-change-note-required This PR does not need a change note label Jan 8, 2025
Copilot AI review requested due to automatic review settings January 8, 2025 15:43
@tausbn tausbn requested a review from a team as a code owner January 8, 2025 15:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)
  • python/ql/src/Metrics/Internal/TypeAnnotations.ql: Language not supported

Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more

@github-actions github-actions bot added the Python label Jan 8, 2025
@RasmusWL RasmusWL self-assigned this Jan 14, 2025
Copy link
Contributor

@joefarebrother joefarebrother left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@RasmusWL RasmusWL left a comment

Choose a reason for hiding this comment

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

Looks fine to me 👍

I ran it on flask, and one thing i immediately did afterwards was to look into the complex type annotations... but it required that I added a any(Annotatable a).getAnnotation() = e. We could consider having those in by default 🤷

Predicates are not following QL style guide with camcelCase, so it seems like you have been too deep in Python land 😅

@tausbn tausbn merged commit a9ab39d into main Mar 6, 2025
9 checks passed
@tausbn tausbn deleted the tausbn/python-add-type-annotation-metrics-query branch March 6, 2025 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants