From da429c88c8d45d2e5222bd8c28d7b75b5b6ab6d4 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Sun, 12 Jul 2015 16:48:00 +0200 Subject: [PATCH] MagicFieldsUtil: refactored and optimized member accessibility check --- .../intellij/nette/utils/MagicFieldsUtil.java | 104 +++++++++++------- 1 file changed, 65 insertions(+), 39 deletions(-) diff --git a/src/cz/juzna/intellij/nette/utils/MagicFieldsUtil.java b/src/cz/juzna/intellij/nette/utils/MagicFieldsUtil.java index bf1c903..05181cc 100644 --- a/src/cz/juzna/intellij/nette/utils/MagicFieldsUtil.java +++ b/src/cz/juzna/intellij/nette/utils/MagicFieldsUtil.java @@ -1,11 +1,7 @@ package cz.juzna.intellij.nette.utils; -import com.intellij.codeInsight.lookup.LookupElement; -import com.intellij.psi.PsiElement; import com.intellij.util.containers.HashMap; import com.jetbrains.php.PhpIndex; -import com.jetbrains.php.completion.PhpVariantsUtil; -import com.jetbrains.php.completion.UsageContext; import com.jetbrains.php.lang.psi.PhpPsiUtil; import com.jetbrains.php.lang.psi.elements.*; import com.jetbrains.php.lang.psi.resolve.types.PhpType; @@ -65,15 +61,9 @@ public static Collection findGetters(FieldReference fieldReference) { if (method == null) { continue; } - ArrayList elements = new ArrayList(); -// elements.add(method); //todo: probably causes recursion, find better way to check visibility Field field = cls.findFieldByName(fieldReference.getName(), false); - if (field != null) { - elements.add(field); - } - Collection accessible = findAccessibleMembers(cls, fieldReference, elements); - - if (!accessible.contains(fieldReference.getName())/* && accessible.contains(method.getName())*/) { + PhpClass containingClass = PhpPsiUtil.getParentByCondition(fieldReference, PhpClass.INSTANCEOF); + if ((field == null || !isAccessible(field, containingClass)) && isAccessible(method, containingClass)) { methods.add(method); } } @@ -85,12 +75,17 @@ public static HashMap> findMagicFields(MemberReferenc HashMap> fields = new HashMap>(); Map classesInFile = cz.juzna.intellij.nette.utils.PhpPsiUtil.getClassesInFile(reference); + PhpClass containingClass = PhpPsiUtil.getParentByCondition(reference, PhpClass.INSTANCEOF); for (PhpClass cls : ClassFinder.getFromMemberReference(reference)) { if (!isNetteObject(cls, classesInFile)) { continue; } - Collection accessibleFields = null; - for (Method method : cls.getMethods()) { + Collection methods = cls.getMethods(); + if (methods.isEmpty()) { + continue; + } + Collection classFields = null; + for (Method method : methods) { String name = method.getName(); String fieldName; if (name.startsWith("get") && name.length() > 3) { @@ -102,10 +97,25 @@ public static HashMap> findMagicFields(MemberReferenc } else { continue; } - if (accessibleFields == null) { - accessibleFields = findAccessibleFields(cls, reference); + if (!isAccessible(method, containingClass)) { + continue; + } + if (classFields == null) { + classFields = cls.getFields(); } - if (accessibleFields.contains(fieldName)) { + boolean isFieldAccessible = false; + for (Field field : classFields) { + if (!field.getName().equals(fieldName)) { + continue; + } + if (isAccessible(field, containingClass)) { + isFieldAccessible = true; + } else { + isFieldAccessible = false; + break; //there is at least one not accessible field + } + } + if (isFieldAccessible) { continue; } @@ -122,28 +132,6 @@ public static HashMap> findMagicFields(MemberReferenc return fields; } - private static Collection findAccessibleFields(PhpClass cls, MemberReference reference) { - return findAccessibleMembers(cls, reference, cls.getFields()); - } - - private static Collection findAccessibleMembers(PhpClass cls, MemberReference reference, Collection elements) { - - PhpClass containingClass = PhpPsiUtil.getParentByCondition(reference, PhpClass.INSTANCEOF); - Collection members = new ArrayList(); - UsageContext usageContext = new UsageContext(PhpModifier.State.PARENT); - usageContext.setTargetObjectClass(cls); - if (containingClass != null) { - usageContext.setClassForAccessFilter(containingClass); - } - - for (LookupElement el : PhpVariantsUtil.getLookupItems(elements, false, usageContext)) { - PsiElement psiEl = el.getPsiElement(); - if (psiEl instanceof PhpNamedElement) { - members.add(((PhpNamedElement) psiEl).getName()); - } - } - return members; - } @NotNull public static HashMap findEventFields(PhpType type, PhpIndex phpIndex) { @@ -166,4 +154,42 @@ private static boolean isNetteObject(PhpClass cls, Map classMa return cz.juzna.intellij.nette.utils.PhpPsiUtil.isTypeOf(cls, "Nette\\Object", classMap); } + private static boolean isAccessible(PhpClassMember member, @Nullable PhpClass accessClass) { + PhpClass elementClass = member.getContainingClass(); + if (classesEqual(elementClass, accessClass)) { + return true; + } + PhpModifier modifier = member.getModifier(); + if (modifier.isPublic()) { + return true; + } + if (accessClass == null || elementClass == null) { + return false; + } + for (PhpClass contextClass = accessClass; contextClass != null; contextClass = contextClass.getSuperClass()) { + if (elementClass.isTrait()) { + for (PhpClass traitClass : contextClass.getTraits()) { + if (classesEqual(elementClass, traitClass)) { + return true; + } + + } + } else if (classesEqual(elementClass, contextClass)) { + return true; + } + if (modifier.isPrivate()) { + break; + } + } + + return false; + } + + + private static boolean classesEqual(@Nullable PhpClass one, @Nullable PhpClass another) { + return one != null && another != null + && one.getFQN() != null && another.getFQN() != null + && one.getFQN().equals(another.getFQN()); + } + }