Skip to content

Re-factor printing of summary component stacks.#13452

Merged
michaelnebel merged 11 commits into
github:mainfrom
michaelnebel:refactorstackprinting
Jul 4, 2023
Merged

Re-factor printing of summary component stacks.#13452
michaelnebel merged 11 commits into
github:mainfrom
michaelnebel:refactorstackprinting

Conversation

@michaelnebel
Copy link
Copy Markdown
Contributor

@michaelnebel michaelnebel commented Jun 14, 2023

In this PR we try to align then printing of summary component stacks with their textual representation as they would like, when being used in an access path (for MaD models).

Furthermore, it turns out that the existing printer getComponentStack is not up to date for all languages. This is also fixed in this PR.

@github-actions github-actions Bot added the C# label Jun 14, 2023
@michaelnebel michaelnebel force-pushed the refactorstackprinting branch from f4c3fcd to 3f06055 Compare June 15, 2023 13:20
@michaelnebel michaelnebel force-pushed the refactorstackprinting branch 3 times, most recently from 3fba6d1 to 87a8117 Compare June 21, 2023 08:08
@michaelnebel michaelnebel requested a review from hvitved June 21, 2023 08:08
@michaelnebel michaelnebel changed the title C#: Re-factor printing of summary component stacks. Re-factor printing of summary component stacks. Jun 21, 2023
Copy link
Copy Markdown
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.

LGTM, just a question about naming.

Comment thread csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll Outdated
Comment thread csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll Outdated
@michaelnebel michaelnebel requested a review from hvitved June 21, 2023 10:36
Copy link
Copy Markdown
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.

LGTM, only minor changes needed. Thanks for doing this 💪

Comment thread ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImplSpecific.qll Outdated
Comment thread ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImplSpecific.qll Outdated
Comment thread swift/ql/test/query-tests/Security/CWE-089/SqlInjection.expected Outdated
@michaelnebel michaelnebel force-pushed the refactorstackprinting branch from ac77e0c to 40e8d33 Compare June 22, 2023 09:36
@michaelnebel michaelnebel added the no-change-note-required This PR does not need a change note label Jun 22, 2023
@michaelnebel michaelnebel requested a review from hvitved June 22, 2023 11:10
@michaelnebel michaelnebel marked this pull request as ready for review June 22, 2023 11:10
@michaelnebel michaelnebel requested review from a team as code owners June 22, 2023 11:10
hvitved
hvitved previously approved these changes Jun 22, 2023
@michaelnebel
Copy link
Copy Markdown
Contributor Author

DCA looks good.
@hvitved : Can you approve again - needed to rebase due to changes in expected output files in swift test cases.

Copy link
Copy Markdown
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

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

I am perhaps slightly confused by the heavy focus on MaD in the QLDoc.
Is the printing not equally aligned with what is used in propagatesFlowExt?

Comment thread csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImplSpecific.qll Outdated
hvitved
hvitved previously approved these changes Jun 23, 2023
@michaelnebel michaelnebel force-pushed the refactorstackprinting branch from 7c3bae1 to 7a5dd8d Compare July 3, 2023 13:45
@michaelnebel michaelnebel marked this pull request as draft July 3, 2023 14:58
@michaelnebel michaelnebel force-pushed the refactorstackprinting branch from 7a5dd8d to 243c592 Compare July 3, 2023 15:01
@michaelnebel michaelnebel marked this pull request as ready for review July 3, 2023 15:05
@michaelnebel
Copy link
Copy Markdown
Contributor Author

@hvitved : Only change since your last review is the last commit.

@michaelnebel michaelnebel merged commit 238f390 into github:main Jul 4, 2023
@michaelnebel michaelnebel deleted the refactorstackprinting branch July 4, 2023 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants