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

LC0052 interfaces #561

Closed
fvet opened this issue Mar 12, 2024 · 8 comments · Fixed by #563 or #572
Closed

LC0052 interfaces #561

fvet opened this issue Mar 12, 2024 · 8 comments · Fixed by #563 or #572
Labels
bug Something isn't working Resolved

Comments

@fvet
Copy link
Contributor

fvet commented Mar 12, 2024

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
Copy link
Contributor Author

fvet commented Mar 12, 2024

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

@Arthurvdv Arthurvdv added the bug Something isn't working label Mar 14, 2024
@Arthurvdv
Copy link
Collaborator

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
Copy link
Collaborator

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

@rvanbekkum
Copy link
Contributor

rvanbekkum commented Mar 17, 2024

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
Copy link
Collaborator

@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
Copy link
Contributor

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
Copy link
Collaborator

@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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Resolved
Projects
None yet
3 participants