Skip to content

Remove unnecessary class from some static accesses#1863

Closed
liblit wants to merge 1 commit intowala:masterfrom
liblit:remove-unnecessary-class-from-some-static-accesses
Closed

Remove unnecessary class from some static accesses#1863
liblit wants to merge 1 commit intowala:masterfrom
liblit:remove-unnecessary-class-from-some-static-accesses

Conversation

@liblit
Copy link
Contributor

@liblit liblit commented Mar 12, 2026

Also, enable the corresponding IntelliJ IDEA inspection to catch similar issues in the future.

Also, enable the corresponding IntelliJ IDEA inspection to catch similar
issues in the future.
@liblit liblit requested a review from msridhar March 12, 2026 22:51
@liblit liblit self-assigned this Mar 12, 2026
@liblit liblit added the cleanup API cleanup and refactoring label Mar 12, 2026
@liblit liblit enabled auto-merge March 12, 2026 22:51
@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

❌ Patch coverage is 38.46154% with 88 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.38%. Comparing base (f92a862) to head (c7ec024).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...a/dalvik/ipa/callgraph/impl/AndroidEntryPoint.java 0.00% 31 Missing ⚠️
...ibm/wala/dalvik/util/AndroidManifestXMLReader.java 0.00% 12 Missing ⚠️
...ava/com/ibm/wala/dalvik/util/AndroidComponent.java 28.57% 4 Missing and 1 partial ⚠️
...a/com/ibm/wala/cfg/exc/intra/NullPointerState.java 70.00% 1 Missing and 2 partials ⚠️
...ibm/wala/cast/ipa/callgraph/CAstCallGraphUtil.java 0.00% 2 Missing ⚠️
...n/java/com/ibm/wala/properties/WalaProperties.java 60.00% 2 Missing ⚠️
...alvik/ipa/callgraph/androidModel/AndroidModel.java 0.00% 2 Missing ⚠️
...main/java/com/ibm/wala/shrike/sourcepos/Debug.java 0.00% 2 Missing ⚠️
...n/java/com/ibm/wala/util/graph/GraphIntegrity.java 0.00% 2 Missing ⚠️
...in/java/com/ibm/wala/util/intset/SparseIntSet.java 0.00% 2 Missing ⚠️
... and 25 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1863      +/-   ##
============================================
+ Coverage     50.37%   50.38%   +0.01%     
- Complexity    12646    12647       +1     
============================================
  Files          1365     1365              
  Lines         84813    84802      -11     
  Branches      14470    14470              
============================================
+ Hits          42725    42729       +4     
+ Misses        37346    37334      -12     
+ Partials       4742     4739       -3     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link

Test Results

  848 files  ±0    848 suites  ±0   5h 25m 34s ⏱️ + 53m 40s
  800 tests ±0    781 ✅ ±0   19 💤 ±0  0 ❌ ±0 
5 484 runs  ±0  5 349 ✅ ±0  135 💤 ±0  0 ❌ ±0 

Results for commit c7ec024. ± Comparison against base commit f92a862.

Copy link
Member

@msridhar msridhar left a comment

Choose a reason for hiding this comment

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

I'm a bit unsure on whether this is a net win in all cases. It's unfortunate that WALA has so many public or protected static fields that get accessed from subclasses in a confusing way; probably the code could be structured better. But, given that we have this, I'm not sure that accessing such fields in an unqualified way is a win, in that it reduces code readability a bit. For a method / field in the same class removing the class qualifier is fine, but I'm not sure about superclasses. In an IDE of course you can just use code navigation to figure out where the method is declared, but I guess we should also be thinking about readability in text editors / for coding agents?

@liblit thoughts?

@liblit
Copy link
Contributor Author

liblit commented Mar 13, 2026

@liblit thoughts?

In general, I favor implicit and concise. But I understand your view too. If you'd rather not make these changes, it's OK with me to close this PR.

@msridhar
Copy link
Member

Let's leave it open for now, I'd like to think it over a bit more.

@msridhar
Copy link
Member

After further thought I think I'd prefer to leave the code as is. I do think it's more readable with the explicit class names in many cases. Thanks for giving this a shot @liblit

@liblit
Copy link
Contributor Author

liblit commented Mar 19, 2026

All good!

@liblit liblit closed this Mar 19, 2026
auto-merge was automatically disabled March 19, 2026 14:48

Pull request was closed

@liblit liblit deleted the remove-unnecessary-class-from-some-static-accesses branch March 19, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup API cleanup and refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants