Skip to content

Conversation

@paldepind
Copy link
Contributor

@paldepind paldepind commented Dec 17, 2025

This PR:

  • Moves member predicates that Struct, Enum, and Union have in common to their common superclass Adt. This makes the class much more useful.
  • Renames the Adt class to TypeItem. Here's a few reasons, that I think TypeItem addresses:
    • The abbreviation is potentially misleading. Our documentation states that it's for "abstract data type", but I'm pretty sure "algebraic data type" is what's intended in the original rust.ungram.
    • Rust types aren't necessarily abstract, it depends on whether fields/variants are made public.
    • I don't think "algebraic data type" is correct either, as union is included which strictly speaking isn't an algebraic type.
    • It's a pretty terse class name.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Dec 17, 2025
private import internal.TypeItemImpl
import codeql.rust.elements.Attr
import codeql.rust.elements.GenericParamList
import codeql.rust.elements.Item

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
codeql.rust.elements.MacroItems
.
import codeql.rust.elements.Attr
import codeql.rust.elements.GenericParamList
import codeql.rust.elements.Item
import codeql.rust.elements.MacroItems

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
codeql.rust.elements.Item
.
@paldepind paldepind force-pushed the rust/adt-class branch 2 times, most recently from 4328d9f to 043d99c Compare December 17, 2025 13:11
@paldepind paldepind changed the title Rust: Tweak Adt class Rust: Improve and rename Adt class Dec 17, 2025
@paldepind paldepind marked this pull request as ready for review December 17, 2025 13:44
@paldepind paldepind requested review from a team as code owners December 17, 2025 13:44
Copilot AI review requested due to automatic review settings December 17, 2025 13:44
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.

Pull request overview

This PR renames the Adt class to TypeItem and refactors it to move common member predicates (attrs, generic_param_list, name, visibility, where_clause) from the individual subclasses (Struct, Enum, Union) to their common superclass. This improves code reusability and makes the class name more descriptive, as it represents items that define types rather than strictly algebraic data types.

Key changes:

  • Renamed Adt class to TypeItem throughout the codebase
  • Moved common predicates from Struct, Enum, and Union to TypeItem superclass
  • Updated database schema relations to consolidate duplicated table definitions
  • Provided upgrade/downgrade scripts for database migration

Reviewed changes

Copilot reviewed 23 out of 40 changed files in this pull request and generated no comments.

Show a summary per file
File Description
rust/schema/ast.py Renamed Adt to TypeItem in base class definitions
rust/schema/annotations.py Added common predicates to TypeItem annotation and dropped them from subclasses
rust/ql/lib/rust.dbscheme Consolidated common relations into type_item_* tables, removed duplicates
rust/ql/lib/upgrades/e54d01f67a416b3d6eb7b970f27295097f2cac7f/* Database upgrade scripts for the schema migration
rust/downgrades/c467bf63916002287b7bde8ce8b094c57579bd81/* Database downgrade scripts for rollback capability
rust/ql/lib/codeql/rust/elements/* Updated QL library files to use new TypeItem class
rust/ql/test/extractor-tests/macro-expansion/test.ql Updated test to use TypeItem instead of Adt
rust/extractor/src/* Updated Rust extractor code to use TypeItem

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

Great change!

Copy link
Contributor

Choose a reason for hiding this comment

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

revert

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants