Skip to content

Java: validate constructor summary models#21415

Open
owen-mc wants to merge 3 commits intogithub:mainfrom
owen-mc:java/validate-constructor-summary-models
Open

Java: validate constructor summary models#21415
owen-mc wants to merge 3 commits intogithub:mainfrom
owen-mc:java/validate-constructor-summary-models

Conversation

@owen-mc
Copy link
Contributor

@owen-mc owen-mc commented Mar 5, 2026

The output column of a summary model for a constructor should use Argument[this] instead of ReturnValue.

@owen-mc owen-mc requested a review from a team as a code owner March 5, 2026 11:56
Copilot AI review requested due to automatic review settings March 5, 2026 11:56
@github-actions github-actions bot added the Java label Mar 5, 2026
@owen-mc owen-mc added the no-change-note-required This PR does not need a change note label Mar 5, 2026
@owen-mc owen-mc force-pushed the java/validate-constructor-summary-models branch from a04cd48 to 579c871 Compare March 5, 2026 12:03
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 encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

Very nice!
Maybe add a QL Doc for the test.
If you intend to introduce the same test for C#, then be aware that the MaD contains type parameters as well.

}

string getIncorrectConstructorSummaryOutput() {
exists(string namespace, string type, string name, string output |
Copy link
Contributor

Choose a reason for hiding this comment

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

nit pick: Consider using package instead of namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's better to move towards using the same variable names in all languages.

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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants