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

Blacklist Google Play Services internal members #23

Merged
merged 4 commits into from
Dec 5, 2017

Conversation

petekanev
Copy link
Contributor

The changes will open the path to fewer typescript errors when typescript-compiling against definitions generated from Google Play Services plugins.

For example after feeding the following android plugins (aars) into the generator, there is only a handful of errors that we need yet to find a way to address:

plugins:

  • android SDK 26
  • appCompat-v7
  • design
  • mediarouter
  • palette
  • recyclerview
  • support-v4
  • transition
  • play-services
  • play-services-ads
  • play-services-auth
  • play-services-vision
  • play-services-tasks
  • play-services-places
  • play-services-identity
  • play-services-location
  • play-services-maps
  • play-services-gcm
  • play-services-basement
  • play-services-nearby

errors list:

android0d5f901e-b5ad-4274-80f8-00cd725bbb2b.d.ts(131890,21): error TS2300: Duplicate identifier 'CastApi'.
android0d5f901e-b5ad-4274-80f8-00cd725bbb2b.d.ts(131890,63): error TS2694: Namespace 'com.google.android.gms.cast.Cast' has no exported member 'CastApi'.
android0d5f901e-b5ad-4274-80f8-00cd725bbb2b.d.ts(131910,20): error TS2300: Duplicate identifier 'CastApi'.
android0d5f901e-b5ad-4274-80f8-00cd725bbb2b.d.ts(131959,21): error TS2300: Duplicate identifier 'CastApi'.
android0d5f901e-b5ad-4274-80f8-00cd725bbb2b.d.ts(131960,92): error TS2694: Namespace 'com.google.android.gms.cast.Cast' has no exported member 'CastApi'.
android0d5f901e-b5ad-4274-80f8-00cd725bbb2b.d.ts(133192,20): error TS2417: Class static side 'typeof CastSession' incorrectly extends base class static side 'typeof Session'.
  Types of property 'zza' are incompatible.
    Type 'typeof com.google.android.gms.cast.framework.CastSession.zza' is not assignable to type 'typeof com.google.android.gms.cast.framework.Session.zza'.
      Property 'joinThreadPool' is missing in type 'typeof zza'.
android0d5f901e-b5ad-4274-80f8-00cd725bbb2b.d.ts(133215,190): error TS2694: Namespace 'com.google.android.gms.cast.Cast' has no exported member 'CastApi'.
android0d5f901e-b5ad-4274-80f8-00cd725bbb2b.d.ts(134599,125): error TS2694: Namespace 'com.google.android.gms.cast.Cast' has no exported member 'CastApi'.
android0d5f901e-b5ad-4274-80f8-00cd725bbb2b.d.ts(173915,56): error TS2694: Namespace 'org.apache.http' has no exported member 'client'.
android0d5f901e-b5ad-4274-80f8-00cd725bbb2b.d.ts(175720,52): error TS2694: Namespace 'org.apache.http' has no exported member 'HttpResponse'.
android0d5f901e-b5ad-4274-80f8-00cd725bbb2b.d.ts(181838,142): error TS2694: Namespace 'com.google.android.gms.cast.Cast' has no exported member 'CastApi'.
android0d5f901e-b5ad-4274-80f8-00cd725bbb2b.d.ts(227143,67): error TS2694: Namespace 'android.support' has no exported member 'customtabs'.

8 out of the 13 errors are one and the same error - a public class has a static public member called CastApi, but the public class also has a nested class of the same name, which is causing the collisions.

@petekanev petekanev requested a review from Plamen5kov November 2, 2017 12:50
Copy link
Contributor

@Plamen5kov Plamen5kov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great, just look at the comments. Some tests would be nice. The most basic ones just running against the most common scenarios: libraries from the Android SDK and making sure we generate an OK .d.ts file ready to use.

@@ -498,8 +504,7 @@ private void writeMethods(Set<String> methodsSet) {
}

private void cacheMethodBySignature(Method m) {
mapNameMethod = new HashMap<String, Method>();
String currMethodSig = getMethodFullSignature(m);
String currMethodSig = m.getName();//getMethodFullSignature(m);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change variable name to better describe its use and remove the comment if it's no longer necessary

@@ -441,6 +445,8 @@ private void generateInterfaceConstructorCommentBlock(JavaClass classInterface,
private void processMethod(Method m, JavaClass clazz, boolean isInterface, Set<String> methodsSet) {
String name = m.getName();

if (name.startsWith("zz")) return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a comment explaining why this is necessary or link to an issue should be sufficient

@@ -613,6 +618,10 @@ private String getMethodParamSignature(JavaClass clazz, Method m) {

//field related
private void processField(Field f, JavaClass clazz) {
String fieldName = f.getName();

if (fieldName.startsWith("zz")) return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

@petekanev
Copy link
Contributor Author

I've got a separate issue open regarding the tests - I'd have to setup a small infrastructure too, and I did not want to pollute this PR - #25

@petekanev petekanev merged commit ca571c9 into master Dec 5, 2017
@petekanev petekanev deleted the pete/blacklist-gps-internals branch December 5, 2017 16:20
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