Skip to content

Java: exclude parameterless static methods from DataFlowTargetApi and from ExternalApi #11717

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

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

jcogs33
Copy link
Contributor

@jcogs33 jcogs33 commented Dec 15, 2022

Description

This PR excludes parameterless static methods from consideration for MaD models, based on a suggestion from @aschackmull. The exclusion is added to the isRelevantForModels predicate for DataFlowTargetApi.
The exclusion is also added to the isUninteresting predicate for ExternalApi.

Consideration

I was going to make this change for C# as well, but I noticed that there seem to be some parameterless static C# methods that should be modeled as sources based on the Telemetry/SupportedExternalSources test cases (namely, System#Console.ReadLine() and System#Console.Read()), so I removed C# from the PR. Let me know if I should add C# back to the PR.
Also, let me know if there would be any similar cases in Java where some parameterless static methods should in fact have models (I couldn't find any examples of this, but I might not have searched long enough).

@jcogs33 jcogs33 added the no-change-note-required This PR does not need a change note label Dec 15, 2022
@github-actions github-actions bot removed the C# label Dec 15, 2022
@jcogs33 jcogs33 changed the title Java/C#: exclude parameterless static methods from DataFlowTargetApi and from ExternalApi Java: exclude parameterless static methods from DataFlowTargetApi and from ExternalApi Dec 15, 2022
@jcogs33 jcogs33 marked this pull request as ready for review December 16, 2022 00:19
@jcogs33 jcogs33 requested a review from a team as a code owner December 16, 2022 00:19
@aschackmull
Copy link
Contributor

Ah, right, such methods can definitely be sources - they just can't have flow through.

@jcogs33
Copy link
Contributor Author

jcogs33 commented Dec 16, 2022

Ah, right, such methods can definitely be sources - they just can't have flow through.

@aschackmull Should I close this PR then?
Or should I look into excluding parameterless static methods only from summaries and not from models in general?

@jcogs33 jcogs33 marked this pull request as draft December 16, 2022 21:36
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.

2 participants