test: avoid method-overwrite warnings in forbidproperties.jl

[PatrickHaecker]

test/forbidproperties.jl is included from six sibling test modules (SparseVectorTests, SparseLinalgTests, SparseTests, SparseIssuesTests, HigherOrderFnsTests, SparseMatrixConstructorIndexingTests). It defines two methods on Base.getproperty for SparseMatrixCSC and SparseVector. With the (hopefully) upcoming Julia default --warn-overwrite=default, the second through sixth includes each produce method-overwrite warnings.

Make the overrides idempotent by only defining them when undefined.

I used Claude Opus 4.7 while preparing this.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.37%. Comparing base (ce5909e) to head (16a9453).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #716      +/-   ##
==========================================
+ Coverage   84.35%   84.37%   +0.02%     
==========================================
  Files          13       13              
  Lines        9376     9376              
==========================================
+ Hits         7909     7911       +2     
+ Misses       1467     1465       -2     

☔ 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.

[dkarrasch]

IIUC then having this is "harmless" regardless of the Julia PR, right? So merging this is safe?

[PatrickHaecker]

IIUC then having this is "harmless" regardless of the Julia PR, right? So merging this is safe?

Yes, exactly. It's already now helping with --warn-overwrite=yes, because the overwrites here are probably not the overwrites the user is interested in.

The reason why this is needed is interesting and I used your question as an opportunity to write it down:

It's basically the #ifndef from C. And we need it because of the same reason as C needs it: The include mechanism on its own is a mechanism to split code, not to share code. However, here we use it for sharing code, which leads to exactly the unwanted behavior we sometimes want to warn about: (accidentally) redefining something.

Luckily, opposed to C, Julia has a more advanced mechanism with its modules. However, includeing a module is still an include, as the module would end up as separate submodules in the (sub)module hierarchy.

So we would need to make it a package to use Julia's even more powerful mechanism. This would work, but at this point we are talking about a larger refactor than I was not comfortable doing as a small PR. It would be cleaner, however.

[dkarrasch]

I think this is really just for internal usage in the tests, so this has nothing to do with the actual productive code. This is used to guard SparseArrays.jl of introducing (specific) getproperty calls, and use accessor functions that then need to be specialized for concrete sparse array subtypes.

[PatrickHaecker]

Yes, exactly. That's why adding this "guard logic" is the pragmatic solution, even if it is not the "cleanest".