Skip to content

Fix Xposed args code generation#2212

Merged
skylot merged 2 commits intoskylot:masterfrom
iscle:xposed-kt-fix
Jul 8, 2024
Merged

Fix Xposed args code generation#2212
skylot merged 2 commits intoskylot:masterfrom
iscle:xposed-kt-fix

Conversation

@iscle
Copy link
Copy Markdown
Contributor

@iscle iscle commented Jul 8, 2024

This PR fixes issues when generating Kotlin Xposed code. The function types were not generated correctly for primitive types and for non common java types.

For example:

  • int was "int.class", which in Kotlin should have been "Int::class.javaPrimitiveType".
  • Unknown types were also like "an.unknown.type.class", which of course does not exist in the Xposed module. It is now just ""an.unknown.type"".

This change also affects generated Java code in that now all, except some specific, classes will be in string form instead of ending in ".class".

The implementation was also converted to Kotlin to make it a bit cleaner and nicer to follow :) I hope you don't mind!

@iscle
Copy link
Copy Markdown
Contributor Author

iscle commented Jul 8, 2024

It would be nice to write some unit tests for this, but I don't know how to do that yet :(
Contributions are welcome!

@iscle iscle changed the title Xposed kt fix Fix Xposed args code generation Jul 8, 2024
@skylot
Copy link
Copy Markdown
Owner

skylot commented Jul 8, 2024

@iscle look good.
A possible minor improvement is to use variables in strings (string interpolation) instead of String.format. And same for logs (with replace logger to kotlin wrapper).

It would be nice to write some unit tests for this

Well, for good test we need actually execute xposed, but this is need emulator and so is too much hassle.
Another way is trying to compile outputs to verify code correctness, this is doable, but I am not sure if this will be helpful.
Also, now xposed and forks repos are in archived state and not active, so I don't think spending time on test will worth it.

@skylot skylot merged commit bbabfa0 into skylot:master Jul 8, 2024
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.

2 participants