Skip to content

Commit

Permalink
* fixed: ObjCProtocol to NSObject promoter in case of private classes…
Browse files Browse the repository at this point in the history
… and multiple constructors (#753)

## Promoter was working only in case one constructor is present.
But in case of private classes there is additional synthetic public constructor is created.
Code updated to ignore synthetic constructors and allow multiple constructors to be available

## Inspection
Added Intellij Idea ispections to highlight following cases:
- if class implements NSObjectProtocol and extends `java.lang.Object` -- warning with quick fix to extend from NSObject (actually this class will be promoted)
- if class extends some class that implements NSObjectProtocol and extends `java.lang.Object` -- warning for both classes and quick fix to navigate to supper that can be updated with NSObject super.
- if class implements protocol but extends class that has some hierarchy behind -- mark as error as this class will not be handled by promoter and will cause Runtime crash
  • Loading branch information
dkimitsa authored Oct 8, 2023
1 parent f8a81f5 commit 6d8bfc9
Show file tree
Hide file tree
Showing 10 changed files with 445 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,11 @@
import org.robovm.compiler.log.Logger;
import org.robovm.compiler.plugin.AbstractCompilerPlugin;
import org.robovm.compiler.plugin.CompilerPlugin;
import soot.Body;
import soot.PatchingChain;
import soot.SootClass;
import soot.SootMethod;
import soot.SootResolver;
import soot.Unit;
import soot.*;
import soot.jimple.InvokeStmt;
import soot.jimple.SpecialInvokeExpr;

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

Expand All @@ -43,6 +39,7 @@ public class ObjCProtocolToObjCObjectPlugin extends AbstractCompilerPlugin {
public static final String OBJC_PROTOCOL = "org.robovm.objc.ObjCProtocol";
public static final String NS_OBJECT = "org.robovm.apple.foundation.NSObject";
public static final String OBJC_OBJECT = "org.robovm.objc.ObjCObject";
private static final int SYNTHETIC = 0x00001000;
private SootClass org_robovm_apple_foundation_NSObject = null;
private SootClass org_robovm_objc_ObjCObject = null;
private SootClass org_robovm_objc_ObjCProtocol = null;
Expand Down Expand Up @@ -106,36 +103,41 @@ private SootClass getSuperReplacementCandidate(SootClass cls) {
* @return true if super call was adjusted
*/
private boolean adjustSuperInitCall(Logger logger, SootClass cls, SootClass newSuper) {
// replacing only java.lang.Object supers and there is expected to be only
// replacing only java.lang.Object super calls
// skipping synthetic constructors and constructors that calls "this".<init> instead of supper
// single kind of super call <init>() so call directly
List<SootMethod> constructors = cls.getMethods().stream()
.filter((m) -> m.getName().equals("<init>"))
.filter((m) -> (m.getModifiers() & SYNTHETIC) == 0 && m.getName().equals("<init>"))
.collect(Collectors.toList());
if (constructors.size() == 1) {
SootMethod method = constructors.get(0);
// getting all expressions for modification
List<SpecialInvokeExpr> expressionsToAdjust = new ArrayList<>();

for (SootMethod method: constructors) {
// find first SpecialInvokeExpr
Body body = method.retrieveActiveBody();
PatchingChain<Unit> units = body.getUnits();
for (Unit unit = units.getFirst(); unit != null; unit = body.getUnits().getSuccOf(unit)) {
if (unit instanceof InvokeStmt) {
InvokeStmt invoke = (InvokeStmt) unit;
if (invoke.getInvokeExpr() instanceof SpecialInvokeExpr) {
// replace only call to Object() constructor
SpecialInvokeExpr expr = (SpecialInvokeExpr) invoke.getInvokeExpr();
if (expr.getMethodRef().getSignature().equals("<java.lang.Object: void <init>()>")) {
expr.setMethodRef(newSuper.getMethod("void <init>()").makeRef());
return true;
}
}
SpecialInvokeExpr expr = units.stream()
.filter(e -> e instanceof InvokeStmt && ((InvokeStmt) e).getInvokeExpr() instanceof SpecialInvokeExpr)
.map(e -> (SpecialInvokeExpr) ((InvokeStmt) e).getInvokeExpr())
.findFirst()
.orElse(null);
if (expr != null) {
if (expr.getMethodRef().getSignature().equals("<java.lang.Object: void <init>()>")) {
expressionsToAdjust.add(expr);
} else if (expr.getMethodRef().declaringClass() != cls) {
// skip "this".<init> calls but fails on other class init call
logger.warn("ObjCProtocol to NSObject failed: missing <java.lang.Object: void <init>()> call in %s", cls.getName());
return false;
}
}

// constructor was not substituted
logger.warn("ObjCProtocol to NSObject failed: missing <java.lang.Object: void <init>()> call in %s", cls.getName());
} else {
logger.warn("ObjCProtocol to NSObject failed: too many public constructors");
}

return false;
// patch collected expression
SootMethodRef replacement = newSuper.getMethod("void <init>()").makeRef();
for (SpecialInvokeExpr expr: expressionsToAdjust)
expr.setMethodRef(replacement);

return true;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package org.robovm.idea.inspection;

import com.intellij.codeInspection.*;
import com.intellij.openapi.project.Project;
import com.intellij.psi.JavaPsiFacade;
import com.intellij.psi.PsiAnonymousClass;
import com.intellij.psi.PsiClass;
import com.intellij.psi.PsiIdentifier;
import org.jetbrains.annotations.NotNull;
import org.robovm.idea.inspection.quickfix.ChangeSuperClassFix;
import org.robovm.idea.inspection.quickfix.NavigateToClassFix;

import java.util.*;

/**
* Simple inspection that check Java objects that implement NSObjectProtocol and
* doesn't extend NSObject
* @see #checkClass(PsiClass, InspectionManager, boolean) for comments
* @author dkimitsa
*/
public class ClassWithProtocolShouldExtendNSObject extends AbstractBaseJavaLocalInspectionTool {
@Override
public ProblemDescriptor[] checkClass(@NotNull PsiClass cls, @NotNull InspectionManager manager, boolean isOnTheFly) {
try {
// ignore: annotations, interfaces and anonymous classes
if (cls instanceof PsiAnonymousClass || cls.isInterface() || cls.isAnnotationType())
return null;

// skip if unable to get class name identifier and build the message
PsiIdentifier classNameIdentifier = cls.getNameIdentifier();
if (classNameIdentifier == null)
return null;

// if extends from NSObject -- do nothing
LinkedList<PsiClass> clsHierarchy = new LinkedList<>();
if (RvmCommonUtils.clsExtends(cls, RvmCommonClassNames.NSObject, clsHierarchy))
return null;

// check hierarchy and if there is any class with @Marshaler annotation -- skip case
for (PsiClass c : clsHierarchy)
if (c.getAnnotation(RvmCommonClassNames.Marshaler) != null)
return null;

// move from bottom of hierarchy to find first class that implements NSObjectProtocol
PsiClass clsWithProtocol = null;
ListIterator<PsiClass> it = clsHierarchy.listIterator(clsHierarchy.size());
while (it.hasPrevious()) {
PsiClass c = it.previous();
if (RvmCommonUtils.clsImplements(c, RvmCommonClassNames.NSObjectProtocol, true)) {
clsWithProtocol = c;
break;
}
}

// if class doesn't implement NSProtocol -- do nothing
if (clsWithProtocol == null || clsWithProtocol.getName() == null)
return null;


ProblemDescriptor[] problemDescriptors = null;
if (clsWithProtocol == clsHierarchy.getLast()) {
// class implements protocol and directly extends from java.lang.Object
// this class will be promoted to NSObject. Attach warning.
if (clsWithProtocol == cls) {
// its same class under analyze, propose switch to NSObject supper as a quick-fix
// show as warning
Project project = cls.getProject();
PsiClass NSObjectCls = JavaPsiFacade.getInstance(project).findClass(RvmCommonClassNames.NSObject,
cls.getResolveScope());
if (NSObjectCls != null) {
LocalQuickFix fix = new ChangeSuperClassFix(NSObjectCls);
String description = RvmInspectionBundle.message("robovm.inspection.objcprotocol.shouldbe.in.nsobject");
ProblemDescriptor p = manager.createProblemDescriptor(classNameIdentifier,
description, fix, ProblemHighlightType.WARNING, false);
problemDescriptors = new ProblemDescriptor[]{p};
}
} else {
// protocol was declared in one of the supers, but its fixable, propose navigate to
// that super as quick fix
// show as warning
LocalQuickFix fix = new NavigateToClassFix(clsWithProtocol, clsWithProtocol.getName());
String description = RvmInspectionBundle.message("robovm.inspection.objcprotocol.parent.should.extend.nsobject",
cls.getName(), clsWithProtocol.getName());
ProblemDescriptor p = manager.createProblemDescriptor(classNameIdentifier,
description, fix, ProblemHighlightType.WARNING, false);
problemDescriptors = new ProblemDescriptor[]{p};
}
} else {
// class has intermediate super between it and java.lang.Object and can't be promoted
// the class can't be promoted and will cause error runtime
String description;
if (cls == clsWithProtocol) {
description = RvmInspectionBundle.message("robovm.inspection.objcprotocol.should.extend.object.or.nsobject.error",
cls.getName(), clsWithProtocol.getName());
} else {
description = RvmInspectionBundle.message("robovm.inspection.objcprotocol.parent.should.extend.object.or.nsobject.error",
cls.getName(), clsWithProtocol.getName());
}
ProblemDescriptor p = manager.createProblemDescriptor(classNameIdentifier,
description, (LocalQuickFix) null, ProblemHighlightType.ERROR, false);
problemDescriptors = new ProblemDescriptor[]{p};
}

return problemDescriptors;
} catch (Exception e) {
throw e;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package org.robovm.idea.inspection;

/**
* set of RoboVM specific class names used for inspection
*/
public final class RvmCommonClassNames {
private RvmCommonClassNames() {}

public static final String NSObject = "org.robovm.apple.foundation.NSObject";
public static final String NSObjectProtocol = "org.robovm.apple.foundation.NSObjectProtocol";
public static final String Marshaler = "org.robovm.rt.bro.annotation.Marshaler";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package org.robovm.idea.inspection;

import com.intellij.psi.*;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.util.List;

public final class RvmCommonUtils {
private RvmCommonUtils() {
}

/**
* Checks if class extends specified class
*
* @param cls to test
* @param classQN qualified name of class to check if extended
*/
public static boolean clsExtends(@NotNull PsiClass cls, @NotNull String classQN) {
return clsExtends(cls, classQN, null);
}

/**
* Checks if class extends specified class
*
* @param cls to test
* @param classQN qualified name of class to check if extended
* @param outHierarchy if specified -- hierarchy of inheritance will be returned
* excluding java.lang.Object and including cls itself
*/
public static boolean clsExtends(@NotNull PsiClass cls, @NotNull String classQN, @Nullable List<PsiClass> outHierarchy) {
if (outHierarchy != null) outHierarchy.add(cls);

PsiClass superCls = cls.getSuperClass();
while (superCls != null && !CommonClassNames.JAVA_LANG_OBJECT.equals(superCls.getQualifiedName())) {
if (outHierarchy != null) outHierarchy.add(superCls);
if (classQN.equals(superCls.getQualifiedName()))
return true;
superCls = superCls.getSuperClass();
}
return false;
}

/**
* checks if class implements Interface
* @param cls to check
* @param interfaceQN fully qualified name of interface
* @param checkSubInterfaces deep check -- super of each interface will be chcekd
* @return true if implements
*/
public static boolean clsImplements(@NotNull PsiClass cls, @NotNull String interfaceQN, boolean checkSubInterfaces) {
PsiClassType[] implementsList = cls.isInterface() ? cls.getExtendsListTypes() : cls.getImplementsListTypes();
for (PsiClassType implType : implementsList) {
if (implType.equalsToText(interfaceQN)) {
return true;
}
if (checkSubInterfaces) {
// check sub-interfaces
PsiClass el = implType.resolve();
if (el != null && clsImplements(el, interfaceQN, true))
return true;
}
}

return false;
}

public static PsiJavaCodeReferenceElement[] getReferences(PsiReferenceList list) {
return list == null ? PsiJavaCodeReferenceElement.EMPTY_ARRAY : list.getReferenceElements();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package org.robovm.idea.inspection;

import com.intellij.DynamicBundle;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.PropertyKey;

import java.util.function.Supplier;

public final class RvmInspectionBundle extends DynamicBundle {
public static final @NonNls String BUNDLE = "robovm.RvmInspectionBundle";

private RvmInspectionBundle() {
super(BUNDLE);
}

private static final DynamicBundle INSTANCE = new RvmInspectionBundle();

public static @NotNull
@Nls String message(@NotNull @PropertyKey(resourceBundle = BUNDLE) String key, Object ... params) {
return INSTANCE.getMessage(key, params);
}

public static @NotNull Supplier<String> messagePointer(@NotNull @PropertyKey(resourceBundle = BUNDLE) String key, Object ... params) {
return INSTANCE.getLazyMessage(key, params);
}
}
Loading

0 comments on commit 6d8bfc9

Please sign in to comment.