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

Replace looseSignature with a better annotation #7770

Conversation

utzcoz
Copy link
Member

@utzcoz utzcoz commented Oct 29, 2022

If one method of a shadow class uses the Class that brought by newer SDK than user's compileSdk, and it will cause compiling error for user because Robolectri's Shadows loads this shadow class, and trigger this shadow class to load this new Class. In previous experience, we use looseSignatures to mark shadow class and use Object to replace all input parameters of this method to avoid compiling error and ensure ShadowWrangle can find this method. looseSignatures work fine now, but it's coarse-grained. ClassName can be used to mark the input parameter that has new Class type only, and ShadowWrangle will provide type checking for real input parameter to ensure real passed input parameters match to expected class types.

Close #6888.

@utzcoz utzcoz force-pushed the looseSignature-find-grained-replacement branch 5 times, most recently from 0734791 to 349e154 Compare October 29, 2022 17:14
@utzcoz
Copy link
Member Author

utzcoz commented Oct 29, 2022

After converting left looseSignature with ClassName as much as possible, I will try to migrate ClassName to the existing WithType as #6884 (comment) suggested.

@utzcoz
Copy link
Member Author

utzcoz commented Oct 29, 2022

@hoisie @brettchabot Although this work is not finished completely, but I think it's time to receive feedback from your experience early.

@hoisie
Copy link
Contributor

hoisie commented Nov 2, 2022

This is great!

@hoisie
Copy link
Contributor

hoisie commented Nov 2, 2022

Checking it now

@hoisie hoisie added the cla: yes label Nov 2, 2022
@hoisie
Copy link
Contributor

hoisie commented Nov 2, 2022

@utzcoz
Copy link
Member Author

utzcoz commented Nov 2, 2022

@hoisie Thanks. I will continue this work with ClassName, but I want to replace it with existing WithType at last. I will check annotation error and try to fix it at next update.

It seems like the sdk validator doesn't run on Gradle?

Looks like not run. I didn't capture this error when running this PR locally and GitHub Actions.

@hoisie
Copy link
Contributor

hoisie commented Nov 2, 2022

You mean the @WithType from reflector? I was hoping to keep the reflector annotations separate from the shadow annotations, even if there was a little bit of overlap. I kind of see reflector as a stand-along library.

@utzcoz
Copy link
Member Author

utzcoz commented Nov 3, 2022

I was hoping to keep the reflector annotations separate from the shadow annotations, even if there was a little bit of overlap.

Okay. I will keep to use ClassName for upcoming changes.

It seems like the sdk validator doesn't run on Gradle?

Which command can be used to run validator? Normal assemble building? We might need to add this checking task on GitHub CI if it doesn't run on CI before.

@utzcoz utzcoz force-pushed the looseSignature-find-grained-replacement branch from 349e154 to b904ce7 Compare November 5, 2022 12:23
@utzcoz utzcoz force-pushed the looseSignature-find-grained-replacement branch from b904ce7 to f368cb4 Compare November 19, 2022 10:31
@utzcoz utzcoz force-pushed the looseSignature-find-grained-replacement branch 2 times, most recently from 8ee4015 to 0ab7c1b Compare January 27, 2024 07:39
@utzcoz utzcoz changed the title Replace looseSignature with ClassName Replace looseSignature with WithType Jan 27, 2024
@utzcoz utzcoz force-pushed the looseSignature-find-grained-replacement branch 2 times, most recently from 6759631 to 5b7900b Compare January 28, 2024 05:23
Signed-off-by: utzcoz <utzcoz@outlook.com>
@utzcoz utzcoz force-pushed the looseSignature-find-grained-replacement branch from 5b7900b to b78b789 Compare February 14, 2024 07:58
@utzcoz utzcoz changed the title Replace looseSignature with WithType Replace looseSignature with a better annotation Feb 14, 2024
@utzcoz utzcoz closed this Feb 14, 2024
@utzcoz utzcoz deleted the looseSignature-find-grained-replacement branch February 14, 2024 07:59
@utzcoz
Copy link
Member Author

utzcoz commented Feb 14, 2024

Moved to #8829 after fixing branch name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate supporting WithType for single input parameter of shadow method
2 participants