NativeScript / android-dts-generator

A tool that generates TypeScript declaration files (.d.ts) from Jars
90 stars 22 forks source link

✨ Emit named method params = better signature help #54

Closed roblav96 closed 3 years ago

roblav96 commented 3 years ago
public constructor(context: android.content.Context, serviceComponent: android.content.ComponentName, callback: android.support.v4.media.MediaBrowserCompat.ConnectionCallback, rootHints: android.os.Bundle);

instead of

public constructor(param0: android.content.Context, param1: android.content.ComponentName, param2: android.support.v4.media.MediaBrowserCompat.ConnectionCallback, param3: android.os.Bundle);

I've tested all tasks in the Makefile with no issues.

cla-bot[bot] commented 3 years ago

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla. CLA has not been signed by users: @roblav96. After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

NathanaelA commented 3 years ago

@roblav96 - Looks good, can you sign the CLA.

roblav96 commented 3 years ago

@NathanaelA This commit 54d5ff5 (#54) resolves a few issues, the primary concern is that on a rare occasion duplicate method definitions are produced yielding the same params type signature but with different names, which is most likely why params0 was originally implemented.

What I've basically done is created a

private Map<String, List<String>> realParams = new HashMap<>();

which stores method params based on it's signature, so that abstract or generic versions of the same method signature can use the params from the original real method param name's instead of arg1 arg2 arg3...

Here's some output examples:

NathanaelA commented 3 years ago

When I get a chance I'm going to test this. Looks like a AWESOME PR; thanks!

NathanaelA commented 3 years ago

Appears to be a bug somewhere in the new code;

Current DTS Generator:

declare module com {
        export module google {
                export module android {
                        export module material {
                                export module transformation {
                                        export class FabTransformationSheetBehavior extends com.google.android.material.transformation.FabTransformationBehavior {
                                                public static class: java.lang.Class<com.google.android.material.transformation.FabTransformationSheetBehavior>;
                                                public constructor();
                                                public onCreateMotionSpec(param0: globalAndroid.content.Context, param1: boolean): com.google.android.material.transformation.FabTransformationBehavior.FabTransformationSpec;
                                                public onExpandedStateChange(param0: globalAndroid.view.View, param1: globalAndroid.view.View, param2: boolean, param3: boolean): boolean;
                                                public constructor(param0: globalAndroid.content.Context, param1: globalAndroid.util.AttributeSet);
                                        }
                                }
                        }
                }
        }
}

With your patch:

declare module com {
        export module google {
                export module android {
                        export module material {
                                export module transformation {
                                        export class FabTransformationSheetBehavior extends com.google.android.material.transformation.FabTransformationBehavior {
                                                public static class: java.lang.Class<com.google.android.material.transformation.FabTransformationSheetBehavior>;
                                                public onCreateMotionSpec(context0: globalAndroid.content.Context, boolean1: boolean): com.google.android.material.transformation.FabTransformationBehavior.FabTransformationSpec;
                                                public constructor();
                                                public onCreateMotionSpec(context: globalAndroid.content.Context, expanded: boolean): com.google.android.material.transformation.FabTransformationBehavior.FabTransformationSpec;
                                                public constructor(context: globalAndroid.content.Context, attrs: globalAndroid.util.AttributeSet);
                                                public onExpandedStateChange(dependency: globalAndroid.view.View, child: globalAndroid.view.View, expanded: boolean, animated: boolean): boolean;
                                        }
                                }
                        }
                }
        }
}

Notice it has two onCreateMotionSpec with almost the same specification but the second one is using number postfix's. In addition the file created has 29834 lines vs 29449 lines for the original DTS generator, so it added another 400ish probably duplicate lines like this one that I am pointing out.

Please note; the comparison is with the current master; and using the cool new test feature make test-compare-output that was merged with master -- it creates a test based on the output of all the jars in the lib folder. Of course with your DTS changes the test will fail (expected); but the number of lines output should match and the classes shouldn't have extra duplicate functions...