StefanMaron / BusinessCentral.LinterCop

Community driven code linter for AL (MS Dynamics 365 Business Central)
https://stefanmaron.com
MIT License
72 stars 30 forks source link

LC0052 interfaces #561

Closed fvet closed 3 months ago

fvet commented 5 months ago

When I declare an internal interface with methods. (Interface is set to internal, allowing use to add methods without breaking things) And implement the interface by adding the methods in a public codeunit, LC0052 pops up. (Codeunit was public due to legacy, so we ended up adding the interface related functions as internal in the codeunit)

image

In my opinion, the LC0052 should not raise a warning here? Functions are not explicitly referenced, but referenced via the interface instead.

fvet commented 5 months ago

The internal method PreProcessIncomingMessage in Codeunit ESCN IC Message Order (Access = Public) is declared but never used.

Arthurvdv commented 5 months ago

You're right, this seems like a false positive.

When a codeunit implements an interface, then LC0052 (and LC0053) should not raise a diagnostic.

Arthurvdv commented 5 months ago

This should now be resolved in the (pre)release version of v0.30.15 of the LinterCop.

rvanbekkum commented 5 months ago

The changes that were made are now leading to false negatives. It should not completely skip procedures in codeunits that implement an interface, because if you do that it skips procedures that are not part of the interface. I think instead the method that checks if a procedure implements an interface procedure could/should be adjusted slightly. 😉

Arthurvdv commented 5 months ago

@rvanbekkum , you're right, thank you for bringing this to our attention.

I need to verify if the procure is declared in the interface and then skip these.

rvanbekkum commented 5 months ago

That's already been accounted for in the current implementation, but maybe there's an issue with the scenario in which the access modifiers of the procedures are not matching? Would need to be checked though, that's just a guess.

Arthurvdv commented 5 months ago

@rvanbekkum, Thanks! That validation was are hiding in plain sight somehow 😅

I've moved this validation a bit up in the chain to early exclude procedures that are part of the interface.

Arthurvdv commented 4 months ago

Version v0.30.16 of the LinterCop is now released. Is it possible to verify of the issue is now resolved?