Skip to content

Propagate named interface assignments of types to return and parameter types of methods declared #1267

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

Conversation

ivangsa
Copy link

@ivangsa ivangsa commented Jun 30, 2025

closes #1264

@ivangsa ivangsa marked this pull request as ready for review June 30, 2025 17:03
@odrotbohm
Copy link
Member

odrotbohm commented Jun 30, 2025

This looks rather involved. I might be missing some detail, but essentially, I was expecting NamedInterface.ofAnnotatedTypes(…) to be extended to not only consider the types themselves but related ones, in case an annotation is found. That should do the trick, shouldn't it? NamedInterfaces shouldn't need to be tweaked at all, I think. We're also gonna need tests.

I'd still love to see a flag on the @NamedInterface annotation that allows restoring the legacy behavior, i.e., something like propagate defaulting to true, which could then be evaluated in the tweaked ….ofAnnotatedTypes(…).

Please let me know if you would want to pursue this further. Totally fine for me to take over, but I guess I'd start from scratch then.

@ivangsa ivangsa force-pushed the feature/namedinterfaces-with-dependencies branch from 2a202a5 to 19a37e0 Compare July 1, 2025 06:57
@ivangsa ivangsa force-pushed the feature/namedinterfaces-with-dependencies branch from 19a37e0 to 1a5649a Compare July 1, 2025 07:00
@ivangsa
Copy link
Author

ivangsa commented Jul 1, 2025

ok, I refactored to just extend NamedInterfaces.ofAnnotatedTypes... this was the first implementation and I got it working earlier and it works for me..

@odrotbohm
Copy link
Member

Thanks again for contributing. I think I'm gonna take this as inspiration to give it a shot myself, as it looks like I'd need to invest the same amount of time course correcting the efforts.

@ivangsa
Copy link
Author

ivangsa commented Jul 1, 2025

this implementation is very simple right now and it doesn't touch any public interface.. I think it's exaclty what you say which was my first approach (bc it covered my use case..)

@ivangsa
Copy link
Author

ivangsa commented Jul 1, 2025

have a look at this test project with the current 1.4.1 implementation... which can not be validated because of related types
https://github.com/ivangsa/modulith-test

I thought (and think) that project should validate... and that would require a much involved implementation bc of the many places NamedInterfaces are build (open, root, named type, named package...)

@odrotbohm
Copy link
Member

Superseded by the fixes made for GH-1264.

@odrotbohm odrotbohm closed this Jul 3, 2025
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.

Propagate named interface assignments of types to return and parameter types of methods declared
2 participants