Skip to content
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

Remove duplicated null and body checks in AbstractCallGraphAlgorithm #1165

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

Conversation

Liyw979
Copy link
Contributor

@Liyw979 Liyw979 commented Jan 23, 2025

No description provided.

@Liyw979
Copy link
Contributor Author

Liyw979 commented Jan 23, 2025

Look like there is something wrong with the CI ...

Error: This request has been automatically failed because it uses a deprecated version of `actions/upload-artifact: v3`. Learn more: https://github.blog/changelog/2024-04-16-deprecation-notice-v3-of-the-artifact-actions/

@JonasKlauke
Copy link
Collaborator

JonasKlauke commented Jan 24, 2025

I get that we can save time by doing the check before, but also these Methods have visibility protected. Shouldnt we provide robust methods as a framework?

@JonasKlauke
Copy link
Collaborator

The null check can be saved if we annote the parameter with nonnull. I'm not sure if we should remove the body check

@swissiety
Copy link
Collaborator

we can kill these nullchecks here as with @Nonnull parameters they just hide bugs originating from somewhere else. Especially when these methods are not public api we should be able to control the originating parts.
Would keep the hasBody unless we make sure even isAbstract() and isNative() will refer to a default/emptyBody.

Copy link

codecov bot commented Mar 21, 2025

Codecov Report

Attention: Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 23.13%. Comparing base (b46c23c) to head (3d43c5d).
Report is 42 commits behind head on develop.

Files with missing lines Patch % Lines
...a/sootup/callgraph/AbstractCallGraphAlgorithm.java 70.00% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##             develop    #1165       +/-   ##
==============================================
- Coverage      72.49%   23.13%   -49.37%     
==============================================
  Files            439      665      +226     
  Lines          17655    28896    +11241     
  Branches        2906     4658     +1752     
==============================================
- Hits           12799     6684     -6115     
- Misses          3789    21653    +17864     
+ Partials        1067      559      -508     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Liyw979
Copy link
Contributor Author

Liyw979 commented Mar 21, 2025

Now we kill null checks using @Nonnull and ifPresent and keep if (!sourceMethod.hasBody()) {

@Liyw979
Copy link
Contributor Author

Liyw979 commented Mar 21, 2025

Why the CI publish-javadoc is failing...

/usr/bin/chmod -R 777 github-pages-deploy-action-temp-deployment-folder
/usr/bin/git worktree remove github-pages-deploy-action-temp-deployment-folder --force
Error: The deploy step encountered an error: The process '/usr/bin/git' failed with exit code 128 ❌
Deployment failed! ❌

Warning: The `set-output` command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

@Liyw979 Liyw979 marked this pull request as draft March 26, 2025 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants