From d49732a8ef1e4c8039624ee8d2b5131db1dfe393 Mon Sep 17 00:00:00 2001 From: Andrei Rosliuk Date: Mon, 20 Mar 2023 09:57:52 +0100 Subject: [PATCH] FixUsesPerformer,ImportDataCreator and UsedNamespaceCollector logic update to fix imports in whole file, fix ModelUtils multiple default namespace detection --- .../php/editor/actions/FixUsesAction.java | 2 +- .../php/editor/actions/FixUsesPerformer.java | 303 ++++++++++-------- .../php/editor/actions/ImportDataCreator.java | 38 ++- .../editor/actions/UsedNamesCollector.java | 72 ++--- .../php/editor/actions/UsedNamespaceName.java | 12 +- .../modules/php/editor/model/ModelUtils.java | 83 +---- .../php/editor/model/impl/ModelBuilder.java | 2 +- ...iple_namespaces_with_multiple_default.pass | 18 +- ...namespaces_with_elements_from_default.pass | 7 - .../testfiles/actions/nb4978_08.php.usedNames | 5 +- .../01/testMultipleDefaultNSUse_01.php | 52 +++ .../testMultipleDefaultNSUse_01.php.fixUses | 54 ++++ .../02/testMultipleDefaultNSUse_02.php | 52 +++ .../testMultipleDefaultNSUse_02.php.fixUses | 54 ++++ .../testfiles/actions/testNB4978/01/Foo.php | 23 ++ .../testfiles/actions/testNB4978/02/Foo.php | 23 ++ .../01/testWholeFileUse_01.php | 52 +++ .../01/testWholeFileUse_01.php.fixUses | 55 ++++ .../02/testWholeFileUse_02.php | 54 ++++ .../02/testWholeFileUse_02.php.fixUses | 58 ++++ ...tiple_namespaces_with_multiple_default.php | 22 +- ..._namespaces_with_elements_from_default.php | 10 - .../editor/actions/FixUsesPerformerTest.java | 29 +- .../modules/php/editor/csl/NavigatorTest.java | 4 - 24 files changed, 794 insertions(+), 290 deletions(-) delete mode 100644 php/php.editor/test/unit/data/goldenfiles/org/netbeans/modules/php/editor/csl/NavigatorTest/structure/php53/multiple_namespaces_with_elements_from_default.pass create mode 100644 php/php.editor/test/unit/data/testfiles/actions/testMultipleDefaultNSUse/01/testMultipleDefaultNSUse_01.php create mode 100644 php/php.editor/test/unit/data/testfiles/actions/testMultipleDefaultNSUse/01/testMultipleDefaultNSUse_01.php.fixUses create mode 100644 php/php.editor/test/unit/data/testfiles/actions/testMultipleDefaultNSUse/02/testMultipleDefaultNSUse_02.php create mode 100644 php/php.editor/test/unit/data/testfiles/actions/testMultipleDefaultNSUse/02/testMultipleDefaultNSUse_02.php.fixUses create mode 100644 php/php.editor/test/unit/data/testfiles/actions/testNB4978/01/Foo.php create mode 100644 php/php.editor/test/unit/data/testfiles/actions/testNB4978/02/Foo.php create mode 100644 php/php.editor/test/unit/data/testfiles/actions/testWholeFileUse/01/testWholeFileUse_01.php create mode 100644 php/php.editor/test/unit/data/testfiles/actions/testWholeFileUse/01/testWholeFileUse_01.php.fixUses create mode 100644 php/php.editor/test/unit/data/testfiles/actions/testWholeFileUse/02/testWholeFileUse_02.php create mode 100644 php/php.editor/test/unit/data/testfiles/actions/testWholeFileUse/02/testWholeFileUse_02.php.fixUses delete mode 100644 php/php.editor/test/unit/data/testfiles/structure/php53/multiple_namespaces_with_elements_from_default.php diff --git a/php/php.editor/src/org/netbeans/modules/php/editor/actions/FixUsesAction.java b/php/php.editor/src/org/netbeans/modules/php/editor/actions/FixUsesAction.java index f3563b91393d..076d077887ed 100644 --- a/php/php.editor/src/org/netbeans/modules/php/editor/actions/FixUsesAction.java +++ b/php/php.editor/src/org/netbeans/modules/php/editor/actions/FixUsesAction.java @@ -155,7 +155,7 @@ private static void setRemoveUnusedUses(final boolean removeUnusedUses) { private static ImportData computeUses(final PHPParseResult parserResult, final int caretPosition) { Map> filteredExistingNames = new UsedNamesCollector(parserResult, caretPosition).collectNames(); Index index = parserResult.getModel().getIndexScope().getIndex(); - NamespaceScope namespaceScope = ModelUtils.getNamespaceScope(parserResult, caretPosition); + NamespaceScope namespaceScope = ModelUtils.getNamespaceScope(parserResult.getModel().getFileScope(), caretPosition); assert namespaceScope != null; ImportData importData = new ImportDataCreator(filteredExistingNames, index, namespaceScope.getNamespaceName(), createOptions(parserResult)).create(); importData.caretPosition = caretPosition; diff --git a/php/php.editor/src/org/netbeans/modules/php/editor/actions/FixUsesPerformer.java b/php/php.editor/src/org/netbeans/modules/php/editor/actions/FixUsesPerformer.java index b0190da50805..ba939280d1b7 100644 --- a/php/php.editor/src/org/netbeans/modules/php/editor/actions/FixUsesPerformer.java +++ b/php/php.editor/src/org/netbeans/modules/php/editor/actions/FixUsesPerformer.java @@ -20,11 +20,11 @@ import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.TreeMap; import java.util.Objects; import java.util.stream.Collectors; import javax.swing.text.BadLocationException; @@ -107,37 +107,34 @@ public void perform() { if (document instanceof BaseDocument) { baseDocument = (BaseDocument) document; editList = new EditList(baseDocument); - namespaceScope = ModelUtils.getNamespaceScope(parserResult, importData.caretPosition); + namespaceScope = ModelUtils.getNamespaceScope(parserResult.getModel().getFileScope(), importData.caretPosition); assert namespaceScope != null; - processSelections(processExistingUses(importData.caretPosition)); + processSelections(); editList.apply(); } } @NbBundle.Messages("FixUsesPerformer.noChanges=Fix imports: No Changes") - private void processSelections(int startOffset) { + private void processSelections() { final List dataItems = resolveDuplicateSelections(); - List useParts = new ArrayList<>(); - Collection declaredGroupUses = namespaceScope.getDeclaredGroupUses(); - for (GroupUseScope groupUseElement : declaredGroupUses) { - for (UseScope useElement : groupUseElement.getUseScopes()) { - processUseElement(useElement, useParts); - } - } - Collection declaredUses = namespaceScope.getDeclaredSingleUses(); - for (UseScope useElement : declaredUses) { - assert !useElement.isPartOfGroupUse() : useElement; - processUseElement(useElement, useParts); - } + TreeMap> usePartsMap = new TreeMap<>(); + for (int i = 0; i < selections.size(); i++) { ItemVariant itemVariant = selections.get(i); - if (itemVariant.canBeUsed()) { + // we shouldn't use itemVariant if there is no any real dataItem related to it + if (itemVariant.canBeUsed() && !dataItems.get(i).getUsedNamespaceNames().isEmpty()) { + NamespaceScope currentScope = dataItems.get(i).getUsedNamespaceNames().get(0).getInScope(); + int mapKey = currentScope.getBlockRange().getStart(); + if (usePartsMap.get(mapKey) == null) { + usePartsMap.put(mapKey, processScopeDeclaredUses(currentScope, new ArrayList<>())); + } + SanitizedUse sanitizedUse = new SanitizedUse( new UsePart(modifyUseName(itemVariant.getName()), UsePart.Type.create(itemVariant.getType()), itemVariant.isFromAliasedElement()), - useParts, - createAliasStrategy(i, useParts, selections)); + usePartsMap.get(mapKey), + createAliasStrategy(i, usePartsMap.get(mapKey), selections)); if (sanitizedUse.shouldBeUsed()) { - useParts.add(sanitizedUse.getSanitizedUsePart()); + usePartsMap.get(mapKey).add(sanitizedUse.getSanitizedUsePart()); } for (UsedNamespaceName usedNamespaceName : dataItems.get(i).getUsedNamespaceNames()) { editList.replace(usedNamespaceName.getOffset(), usedNamespaceName.getReplaceLength(), sanitizedUse.getReplaceName(usedNamespaceName), false, 0); @@ -145,13 +142,157 @@ private void processSelections(int startOffset) { } } replaceUnimportedItems(); - String insertString = createInsertString(useParts, shouldAppendLineAfter(startOffset)); - // avoid being recognized as a modified file - if (insertString.isEmpty()) { + + CheckVisitor visitor = new CheckVisitor(); + Program program = parserResult.getProgram(); + if (program != null) { + program.accept(visitor); + } + + boolean emptyString = true; + int lastUsedRangeIndex = 0; + int lastDeclareIndex = 0; + List declaredNamespaces; + if (namespaceScope.isDefaultNamespace()) { + declaredNamespaces = new ArrayList(namespaceScope.getFileScope().getDeclaredNamespaces()); + declaredNamespaces.sort((left, right) -> left.getBlockRange().getStart() - right.getBlockRange().getStart()); + } else { + declaredNamespaces = new ArrayList(); + declaredNamespaces.add(namespaceScope); + } + for (NamespaceScope currentScope : declaredNamespaces) { + int mapKey = currentScope.getBlockRange().getStart(); + if (usePartsMap.get(mapKey) == null) { + usePartsMap.put(mapKey, processScopeDeclaredUses(currentScope, new ArrayList<>())); + } + + int startOffset = getNamespaceScopeOffset(currentScope); + int endOffset = currentScope.isDefaultNamespace() && visitor.getGlobalNamespaceEndOffset() > 0 + ? visitor.getGlobalNamespaceEndOffset() + : currentScope.getBlockRange().getEnd(); + + int compareStringOffsetStart = 0; + List replace = new ArrayList(); + for (int i = lastUsedRangeIndex; i < visitor.getUsedRanges().size(); i++) { + OffsetRange offsetRange = visitor.getUsedRanges().get(i); + if (endOffset < offsetRange.getStart()) { + break; + } + lastUsedRangeIndex = i; + if (startOffset > offsetRange.getStart()) { + continue; + } + + compareStringOffsetStart = compareStringOffsetStart > 0 ? compareStringOffsetStart : offsetRange.getStart(); + int useStartOffset = getOffsetWithoutLeadingWhitespaces(offsetRange.getStart()); + replace.add(new OffsetRange(useStartOffset, offsetRange.getEnd())); + startOffset = offsetRange.getEnd(); + } + + // because only declare(strict_types=1) should go first, but declare(ticks=1) could be everywhere + // we have to restrict declare start offset to prevent wrong USE placement after declare in cases such + // function { declare(ticks=1); } or class A { puclic function fn () { declare(ticks=1); } } + // in the feature it would be better to have DeclareStatmentScope for that + int maxDeclareOffset = currentScope.getElements().isEmpty() ? endOffset : currentScope.getElements().get(0).getOffset(); + // NETBEANS-4978 check whether declare statemens exist + // e.g. in the following case, insert the code(use statements) after the declare statement + // declare (strict_types=1); + // class TestClass + // { + // public function __construct() + // { + // $test = new Foo(); + // } + // } + for (int j = lastDeclareIndex; j < visitor.getDeclareStatements().size(); j++) { + if (maxDeclareOffset < visitor.getDeclareStatements().get(j).getStartOffset()) { + break; + } + startOffset = Math.max(startOffset, visitor.getDeclareStatements().get(j).getEndOffset()); + lastDeclareIndex = j; + } + + String insertString = createInsertString(usePartsMap.get(mapKey)); + // avoid being recognized as a modified file + if (insertString.isEmpty()) { + // remove unused namespaces if exists + for (OffsetRange offsetRange : replace) { + editList.replace(offsetRange.getStart(), offsetRange.getLength(), EMPTY_STRING, false, 0); + emptyString = false; + } + continue; + } + + try { + // get -2/+2 for new lines before string + String replaceString = compareStringOffsetStart > 1 ? baseDocument.getText(compareStringOffsetStart - 2, startOffset - compareStringOffsetStart + 2) : ""; + if (replaceString.equals(insertString)) { + continue; + } + + for (OffsetRange offsetRange : replace) { + editList.replace(offsetRange.getStart(), offsetRange.getLength(), EMPTY_STRING, false, 0); + } + editList.replace(startOffset, 0, insertString, false, 0); + if (shouldAppendLineAfter(startOffset)) { + editList.replace(startOffset, 0, NEW_LINE, false, 0); + } + emptyString = false; + } catch (BadLocationException ex) { + Exceptions.printStackTrace(ex); + } + } + + if (emptyString) { StatusDisplayer.getDefault().setStatusText(Bundle.FixUsesPerformer_noChanges()); + } + } + + private List processScopeDeclaredUses(NamespaceScope namespaceScope, List useParts) { + for (GroupUseScope groupUseElement : namespaceScope.getDeclaredGroupUses()) { + for (UseScope useElement : groupUseElement.getUseScopes()) { + processUseElement(useElement, useParts); + } + } + for (UseScope useElement : namespaceScope.getDeclaredSingleUses()) { + assert !useElement.isPartOfGroupUse() : useElement; + processUseElement(useElement, useParts); + } + return useParts; + } + + private int getNamespaceScopeOffset(NamespaceScope scope) { + int useScopeStart = scope.getBlockRange().getStart(); + if (useScopeStart == 0) { + // this is check for case when we don't have namespace declaration in file + // such ts = LexUtilities.getPositionedSequence(baseDocument, scope.getBlockRange().getEnd()); + if (ts != null) { + LexUtilities.findPreviousToken(ts, Arrays.asList(PHPTokenId.PHP_OPENTAG, PHPTokenId.PHPDOC_COMMENT_END)); + if (ts.token().id().equals(PHPTokenId.PHP_OPENTAG) || ts.token().id().equals(PHPTokenId.PHPDOC_COMMENT_END)) { + useScopeStart = ts.offset() + ts.token().length() + 1; + } + } + } finally { + baseDocument.readUnlock(); + } } else { - editList.replace(startOffset, 0, insertString, false, 0); + //because when semicolon in the end of a namespace, the block starts + //after semicolon, but for brace it starts before curly open + //we can't just add +1 because of such case + baseDocument.readLock(); + try { + if (LexUtilities.getTokenChar(baseDocument, useScopeStart) == CURLY_OPEN) { + useScopeStart++; + } + } finally { + baseDocument.readUnlock(); + } } + return useScopeStart; } private boolean shouldAppendLineAfter(int lineOffset) { @@ -185,6 +326,7 @@ private List resolveDuplicateSelections() { for (int j = i + 1; j < selectionsCopy.size(); j++) { ItemVariant testedVariant = selectionsCopy.get(j); if (baseVariant.equals(testedVariant) + && dataItems.get(j).getUsedNamespaceNames().get(0).getInScope() == dataItems.get(i).getUsedNamespaceNames().get(0).getInScope() && itemIndexesToRemove.add(j)) { ImportData.DataItem duplicateItem = dataItems.get(j); usedNamespaceNames.addAll(duplicateItem.getUsedNamespaceNames()); @@ -250,7 +392,7 @@ private boolean isUsed(final UseScope useElement) { return result; } - private String createInsertString(final List useParts, boolean shouldAppendLineAfter) { + private String createInsertString(final List useParts) { StringBuilder insertString = new StringBuilder(); Collections.sort(useParts); if (useParts.size() > 0) { @@ -271,9 +413,6 @@ private String createInsertString(final List useParts, boolean shouldAp } else { insertString.append(createStringForCommonUse(useParts)); } - if (shouldAppendLineAfter) { - insertString.append(NEW_LINE); - } return insertString.toString(); } @@ -420,106 +559,6 @@ private String createStringForCommonUse(List useParts) { return result.toString(); } - private int processExistingUses(int caretPosition) { - CheckVisitor visitor = new CheckVisitor(); - Program program = parserResult.getProgram(); - if (program != null) { - program.accept(visitor); - } - - // import has been working properly only for first default namespace - // or for namespace under caret, so we should not remove USES in other - // namespaces in the same file and use only current scope - int useScopeStart = namespaceScope.getBlockRange().getStart(); - int useScopeEnd = namespaceScope.getBlockRange().getEnd(); - - List globalNamespaceDeclarations = visitor.getGlobalNamespaceDeclarations(); - if (namespaceScope.isDefaultNamespace()) { - // this is check for case when we don't have namespace declaration in file - // such ts = LexUtilities.getPositionedSequence(baseDocument, namespaceScope.getBlockRange().getEnd()); - if (ts != null) { - LexUtilities.findPreviousToken(ts, Arrays.asList(PHPTokenId.PHP_OPENTAG, PHPTokenId.PHPDOC_COMMENT_END)); - if (ts.token().id().equals(PHPTokenId.PHP_OPENTAG) || ts.token().id().equals(PHPTokenId.PHPDOC_COMMENT_END)) { - useScopeStart = ts.offset() + ts.token().length() + 1; - } - } - } finally { - baseDocument.readUnlock(); - } - } else { - // only for case when caret outside of global namespace nested - // in defalut global namespace such as 0) { - //because when semicolon in the end of a namespace, the block starts - //after semicolon, but for brace it starts before curly open - //we can't just add +1 because of such case - baseDocument.readLock(); - try { - if (LexUtilities.getTokenChar(baseDocument, useScopeStart) == CURLY_OPEN) { - useScopeStart++; - } - } finally { - baseDocument.readUnlock(); - } - } - - int lastUseOffset = useScopeStart; - for (OffsetRange offsetRange : visitor.getUsedRanges()) { - if (offsetRange.getStart() < useScopeStart || offsetRange.getEnd() > useScopeEnd) { - continue; - } - int startOffset = getOffsetWithoutLeadingWhitespaces(offsetRange.getStart()); - editList.replace(startOffset, offsetRange.getEnd() - startOffset, EMPTY_STRING, false, 0); - lastUseOffset = offsetRange.getEnd() > lastUseOffset ? offsetRange.getEnd() : lastUseOffset; - } - - // because only declare(strict_types=1) should go first, but declare(ticks=1) could be everywhere - // we have to restrict declare start offset to prevent wrong USE placement after declare in cases such - // function { declare(ticks=1); } or class A { puclic function fn () { declare(ticks=1); } } - // in the feature it would be better to have DeclareStatmentScope for that - int maxDeclareOffset = namespaceScope.getElements().isEmpty() ? useScopeEnd : namespaceScope.getElements().get(0).getOffset(); - // NETBEANS-4978 check whether declare statemens exist - // e.g. in the following case, insert the code(use statements) after the declare statement - // declare (strict_types=1); - // class TestClass - // { - // public function __construct() - // { - // $test = new Foo(); - // } - // } - for (DeclareStatement declareStatement : visitor.getDeclareStatements()) { - if (maxDeclareOffset < declareStatement.getStartOffset()) { - break; - } - lastUseOffset = Math.max(lastUseOffset, declareStatement.getEndOffset()); - } - - - return lastUseOffset; - } - private int getOffsetWithoutLeadingWhitespaces(final int startOffset) { int result = startOffset; baseDocument.readLock(); @@ -543,6 +582,7 @@ private static class CheckVisitor extends DefaultVisitor { private List declareStatements = new ArrayList<>(); private List globalNamespaceDeclarations = new ArrayList<>(); private final List usedRanges = new LinkedList<>(); + private int globalNamespaceEndOffset = 0; public List getDeclareStatements() { return Collections.unmodifiableList(declareStatements); @@ -555,6 +595,10 @@ public List getGlobalNamespaceDeclarations() { public List getUsedRanges() { return Collections.unmodifiableList(usedRanges); } + + public int getGlobalNamespaceEndOffset() { + return globalNamespaceEndOffset; + } @Override public void visit(UseStatement node) { @@ -579,6 +623,9 @@ public void visit(NamespaceDeclaration declaration) { if (CancelSupport.getDefault().isCancelled()) { return; } + if (globalNamespaceEndOffset == 0 || globalNamespaceEndOffset > declaration.getBody().getEndOffset()) { + globalNamespaceEndOffset = declaration.getBody().getStartOffset(); + } if (declaration.isBracketed() && declaration.getName() == null) { globalNamespaceDeclarations.add(declaration); } diff --git a/php/php.editor/src/org/netbeans/modules/php/editor/actions/ImportDataCreator.java b/php/php.editor/src/org/netbeans/modules/php/editor/actions/ImportDataCreator.java index c205d02a7e09..eb0789fc7ae6 100644 --- a/php/php.editor/src/org/netbeans/modules/php/editor/actions/ImportDataCreator.java +++ b/php/php.editor/src/org/netbeans/modules/php/editor/actions/ImportDataCreator.java @@ -27,7 +27,8 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.TreeSet; +import java.util.HashMap; +import java.util.TreeMap; import org.netbeans.modules.php.api.PhpVersion; import org.netbeans.modules.php.editor.actions.FixUsesAction.Options; import org.netbeans.modules.php.editor.actions.ImportData.DataItem; @@ -74,8 +75,22 @@ public ImportDataCreator(final Map> usedNames, f } public ImportData create() { - for (String fqElementName : new TreeSet<>(usedNames.keySet())) { - processFQElementName(fqElementName); + for (Map.Entry> entry : (new TreeMap<>(usedNames)).entrySet()) { + if (entry.getValue().size() > 1) { + Map> scopeNames = new HashMap(); + for (UsedNamespaceName usedName : entry.getValue()) { + Integer scopeOffset = usedName.getInScope().getBlockRange().getStart(); + if (!scopeNames.containsKey(scopeOffset)) { + scopeNames.put(scopeOffset, new ArrayList()); + } + scopeNames.get(scopeOffset).add(usedName); + } + for (Map.Entry> keyNames : scopeNames.entrySet()) { + processFQElementName(entry.getKey(), keyNames.getValue()); + } + } else { + processFQElementName(entry.getKey(), entry.getValue()); + } } ImportData data = new ImportData(); for (PossibleItem possibleItem : possibleItems) { @@ -85,7 +100,7 @@ public ImportData create() { return data; } - private void processFQElementName(final String fqElementName) { + private void processFQElementName(final String fqElementName, List usedNames) { Collection possibleFQElements = fetchPossibleFQElements(fqElementName); Collection filteredPlatformConstsAndFunctions = filterPlatformConstsAndFunctions(possibleFQElements); Collection filteredDuplicates = filterDuplicates(filteredPlatformConstsAndFunctions); @@ -101,10 +116,11 @@ private void processFQElementName(final String fqElementName) { } else { Collection filteredFQElements = filterFQElementsFromCurrentNamespace(filteredExactUnqualifiedNames); if (filteredFQElements.isEmpty()) { - possibleItems.add(new ReplaceItem(fqElementName, filteredExactUnqualifiedNames)); + possibleItems.add(new ReplaceItem(fqElementName, usedNames, filteredExactUnqualifiedNames)); } else { possibleItems.add(new ValidItem( fqElementName, + usedNames, filteredFQElements, filteredFQElements.size() != filteredExactUnqualifiedNames.size())); } @@ -226,10 +242,12 @@ public void insertData(ImportData data) { private final class ReplaceItem implements PossibleItem { private final String fqName; + private final List usedNames; private final Collection filteredExactUnqualifiedNames; - public ReplaceItem(String fqName, Collection filteredExactUnqualifiedNames) { + public ReplaceItem(String fqName, List usedNames, Collection filteredExactUnqualifiedNames) { this.fqName = fqName; + this.usedNames = usedNames; this.filteredExactUnqualifiedNames = filteredExactUnqualifiedNames; } @@ -241,7 +259,7 @@ public void insertData(ImportData data) { ? fqElement.getFullyQualifiedName().toString() : fqElement.getName(); ItemVariant replaceItemVariant = new ItemVariant(itemVariantReplaceName, ItemVariant.UsagePolicy.CAN_BE_USED); - data.addJustToReplace(new DataItem(fqName, Collections.singletonList(replaceItemVariant), replaceItemVariant, usedNames.get(fqName))); + data.addJustToReplace(new DataItem(fqName, Collections.singletonList(replaceItemVariant), replaceItemVariant, usedNames)); } private FullyQualifiedElement findBestElement() { @@ -258,10 +276,12 @@ private FullyQualifiedElement findBestElement() { private final class ValidItem implements PossibleItem { private final Collection filteredFQElements; private final String typeName; + private final List usedNames; private final boolean existsFQElementFromCurrentNamespace; - private ValidItem(String typeName, Collection filteredFQElements, boolean existsFQELEMENTFromCurrentNamespace) { + private ValidItem(String typeName, List usedNames, Collection filteredFQElements, boolean existsFQELEMENTFromCurrentNamespace) { this.typeName = typeName; + this.usedNames = usedNames; this.filteredFQElements = filteredFQElements; this.existsFQElementFromCurrentNamespace = existsFQELEMENTFromCurrentNamespace; } @@ -306,7 +326,7 @@ public void insertData(ImportData data) { } } Collections.sort(variants); - data.add(new DataItem(typeName, variants, defaultValue, usedNames.get(typeName))); + data.add(new DataItem(typeName, variants, defaultValue, usedNames)); } } diff --git a/php/php.editor/src/org/netbeans/modules/php/editor/actions/UsedNamesCollector.java b/php/php.editor/src/org/netbeans/modules/php/editor/actions/UsedNamesCollector.java index d3286951f768..3e4d9d0c88ac 100644 --- a/php/php.editor/src/org/netbeans/modules/php/editor/actions/UsedNamesCollector.java +++ b/php/php.editor/src/org/netbeans/modules/php/editor/actions/UsedNamesCollector.java @@ -22,6 +22,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -35,7 +36,6 @@ import org.netbeans.modules.php.editor.model.impl.Type; import org.netbeans.modules.php.editor.parser.PHPParseResult; import org.netbeans.modules.php.editor.parser.astnodes.ASTNode; -import org.netbeans.modules.php.editor.parser.astnodes.Block; import org.netbeans.modules.php.editor.parser.astnodes.NamespaceDeclaration; import org.netbeans.modules.php.editor.parser.astnodes.NamespaceName; import org.netbeans.modules.php.editor.parser.astnodes.PHPDocTypeNode; @@ -65,56 +65,19 @@ public UsedNamesCollector(final PHPParseResult parserResult, final int caretPosi } public Map> collectNames() { - NamespaceScope namespaceScope = ModelUtils.getNamespaceScope(parserResult, caretPosition); + NamespaceScope namespaceScope = ModelUtils.getNamespaceScope(parserResult.getModel().getFileScope(), caretPosition); assert namespaceScope != null; OffsetRange offsetRange = namespaceScope.getBlockRange(); - // in the following case, avoid being inserted incorrect uses - // because default namespace range is whole file... - // // caret here^ - // declare (strict_types=1); - // - // namespace { - // class GlobalNamespace {} - // } - // - // namespace Test { - // class TestClass { - // public function __construct() - // { - // $test = new Foo(); - // } - // public function test(?Foo $foo): ?Foo - // { - // return null; - // } - // } - // } - if (namespaceScope.isDefaultNamespace()) { - NamespaceDeclarationVisitor namespaceDeclarationVisitor = new NamespaceDeclarationVisitor(); - parserResult.getProgram().accept(namespaceDeclarationVisitor); - List globalNamespaces = namespaceDeclarationVisitor.getGlobalNamespaceDeclarations(); - if (!globalNamespaces.isEmpty()) { - Block body = globalNamespaces.get(0).getBody(); - offsetRange = new OffsetRange(body.getStartOffset(), body.getEndOffset()); - } - for (NamespaceDeclaration globalNamespace : globalNamespaces) { - if (globalNamespace.getBody().getStartOffset() <= caretPosition - && caretPosition <= globalNamespace.getBody().getEndOffset()) { - offsetRange = new OffsetRange(globalNamespace.getBody().getStartOffset(), globalNamespace.getBody().getEndOffset()); - } - } - } - Collection declaredUses = namespaceScope.getAllDeclaredSingleUses(); - NamespaceNameVisitor namespaceNameVisitor = new NamespaceNameVisitor(offsetRange); + NamespaceNameVisitor namespaceNameVisitor = new NamespaceNameVisitor(offsetRange, namespaceScope); parserResult.getProgram().accept(namespaceNameVisitor); possibleNames = namespaceNameVisitor.getExistingNames(); - return filterNamesWithoutUses(declaredUses); + return filterNamesWithoutUses(namespaceNameVisitor.getScopeMap()); } - private Map> filterNamesWithoutUses(final Collection declaredUses) { - final Map> result = new HashMap<>(); + private Map> filterNamesWithoutUses(final Map scopeMap) { + final Map> result = new LinkedHashMap<>(); for (Map.Entry> entry : possibleNames.entrySet()) { - if (!existsUseForTypeName(declaredUses, QualifiedName.create(entry.getKey()))) { + if (!existsUseForTypeName(scopeMap.get(entry.getKey()).getAllDeclaredSingleUses(), QualifiedName.create(entry.getKey()))) { result.put(entry.getKey(), entry.getValue()); } } @@ -143,10 +106,14 @@ private boolean existsUseForTypeName(final Collection declar private static class NamespaceNameVisitor extends DefaultVisitor { private final OffsetRange offsetRange; - private final Map> existingNames = new HashMap<>(); + private final NamespaceScope namespaceScope; + private NamespaceScope currentScope; + private final Map scopeMap = new HashMap<>(); + private final Map> existingNames = new LinkedHashMap<>(); - public NamespaceNameVisitor(OffsetRange offsetRange) { + public NamespaceNameVisitor(OffsetRange offsetRange, NamespaceScope namespaceScope) { this.offsetRange = offsetRange; + this.namespaceScope = namespaceScope; } @Override @@ -166,18 +133,23 @@ private boolean isInNamespace(ASTNode node) { @Override public void visit(Program node) { + // just for safety to reset initial scope on a next run + currentScope = namespaceScope; scan(node.getStatements()); scan(node.getComments()); } @Override public void visit(NamespaceDeclaration node) { + if (namespaceScope.isDefaultNamespace()) { + currentScope = ModelUtils.getNamespaceScope(namespaceScope.getFileScope(), node.getBody().getStartOffset()); + } scan(node.getBody()); } @Override public void visit(NamespaceName node) { - UsedNamespaceName usedName = new UsedNamespaceName(node); + UsedNamespaceName usedName = new UsedNamespaceName(node, currentScope); if (isValidTypeName(usedName.getName())) { processUsedName(usedName); } @@ -185,7 +157,7 @@ public void visit(NamespaceName node) { @Override public void visit(PHPDocTypeNode node) { - UsedNamespaceName usedName = new UsedNamespaceName(node); + UsedNamespaceName usedName = new UsedNamespaceName(node, currentScope); if (isValidTypeName(usedName.getName()) && isValidAliasTypeName(usedName.getName())) { processUsedName(usedName); } @@ -204,6 +176,7 @@ private void processUsedName(final UsedNamespaceName usedName) { if (usedNames == null) { usedNames = new LinkedList<>(); existingNames.put(usedName.getName(), usedNames); + scopeMap.put(usedName.getName(), usedName.getInScope()); } usedNames.add(usedName); } @@ -212,6 +185,9 @@ public Map> getExistingNames() { return Collections.unmodifiableMap(existingNames); } + public Map getScopeMap() { + return Collections.unmodifiableMap(scopeMap); + } } private static class NamespaceDeclarationVisitor extends DefaultVisitor { diff --git a/php/php.editor/src/org/netbeans/modules/php/editor/actions/UsedNamespaceName.java b/php/php.editor/src/org/netbeans/modules/php/editor/actions/UsedNamespaceName.java index ea23cd764b1b..c3e7a8da9d4d 100644 --- a/php/php.editor/src/org/netbeans/modules/php/editor/actions/UsedNamespaceName.java +++ b/php/php.editor/src/org/netbeans/modules/php/editor/actions/UsedNamespaceName.java @@ -19,6 +19,7 @@ package org.netbeans.modules.php.editor.actions; import org.netbeans.modules.php.editor.api.QualifiedName; +import org.netbeans.modules.php.editor.model.NamespaceScope; import org.netbeans.modules.php.editor.parser.astnodes.ASTNode; import org.netbeans.modules.php.editor.parser.astnodes.NamespaceName; import org.netbeans.modules.php.editor.parser.astnodes.PHPDocTypeNode; @@ -29,21 +30,28 @@ */ public class UsedNamespaceName { private final ASTNode node; + private final NamespaceScope inScope; private final QualifiedName qualifiedName; - public UsedNamespaceName(NamespaceName node) { + public UsedNamespaceName(NamespaceName node, NamespaceScope inScope) { this.node = node; + this.inScope = inScope; this.qualifiedName = QualifiedName.create(node); } - public UsedNamespaceName(PHPDocTypeNode node) { + public UsedNamespaceName(PHPDocTypeNode node, NamespaceScope inScope) { this.node = node; + this.inScope = inScope; this.qualifiedName = QualifiedName.create(node.getValue()); } public int getOffset() { return node.getStartOffset(); } + + public NamespaceScope getInScope() { + return inScope; + } public int getReplaceLength() { return qualifiedName.toString().length(); diff --git a/php/php.editor/src/org/netbeans/modules/php/editor/model/ModelUtils.java b/php/php.editor/src/org/netbeans/modules/php/editor/model/ModelUtils.java index 9ed619c28c31..af8a561ae732 100644 --- a/php/php.editor/src/org/netbeans/modules/php/editor/model/ModelUtils.java +++ b/php/php.editor/src/org/netbeans/modules/php/editor/model/ModelUtils.java @@ -93,9 +93,22 @@ public static Set getAliasedNames(final Model model, final int offs public static NamespaceScope getNamespaceScope(NamespaceDeclaration currenNamespace, FileScope fileScope) { NamespaceDeclarationInfo ndi = currenNamespace != null ? NamespaceDeclarationInfo.create(currenNamespace) : null; - NamespaceScope currentScope = ndi != null - ? ModelUtils.getFirst(ModelUtils.filter(fileScope.getDeclaredNamespaces(), ndi.getName())) - : fileScope.getDefaultDeclaredNamespace(); + NamespaceScope currentScope; + if (ndi != null) { + // we could have many unnamed namespaces, so should select right one + final int currentNamespaceNameStartOffset = ndi.getRange().getStart(); + currentScope = ndi.isDefaultNamespace() + ? ModelUtils.getFirst(filter(fileScope.getDeclaredNamespaces(), new ElementFilter() { + @Override + public boolean isAccepted(NamespaceScope element) { + return element.getNameRange().getStart() == currentNamespaceNameStartOffset; + } + })) + : ModelUtils.getFirst(ModelUtils.filter(fileScope.getDeclaredNamespaces(), ndi.getName())); + } else { + currentScope = fileScope.getDefaultDeclaredNamespace(); + } + return currentScope; } @@ -435,70 +448,6 @@ public static NamespaceScope getNamespaceScope(FileScope fileScope, int offset) return retval; } - public static NamespaceScope getNamespaceScope(PHPParseResult parserResult, int offset) { - // NETBEANS-4978 - // if the caret position is not in each namespace scope, get the first namespace scope - // e.g. - // [original] - // // caret is here^ - // declare (strict_types=1); - // namespace Foo; - // ... - // namespace Bar; - // - // [inserted] - // // caret is here - // declare (strict_types=1); - // namespace Foo; - // use ...; - // ... - // namespace Bar; - FileScope fileScope = parserResult.getModel().getFileScope(); - List namespaceDeclarations = getNamespaceDeclarations(parserResult); - boolean isBracketed = false; - if (!namespaceDeclarations.isEmpty()) { - isBracketed = namespaceDeclarations.get(0).isBracketed(); - } - NamespaceScope retval = ModelUtils.getNamespaceScope(fileScope, offset); - assert retval != null; - Collection declaredNamespaces = fileScope.getDeclaredNamespaces(); - if (retval != null && retval.isDefaultNamespace()) { - // get the first namespace scope - if (isBracketed) { - for (NamespaceDeclaration namespaceDeclaration : namespaceDeclarations) { - if (namespaceDeclaration.getName() == null - && namespaceDeclaration.getStartOffset() <= offset && offset <= namespaceDeclaration.getEndOffset()) { - return retval; - } - } - if (namespaceDeclarations.get(0).getName() == null) { - return retval; - } - retval = getFirstNamespaceScope(declaredNamespaces, retval); - } - if (!isBracketed && !namespaceDeclarations.isEmpty()) { - // if namespace is not bracketed, - // both the global namespace and another namespace don't exist in the same file - retval = getFirstNamespaceScope(declaredNamespaces, retval); - } - } - return retval; - } - - private static NamespaceScope getFirstNamespaceScope(Collection declaredNamespaces, NamespaceScope namespace) { - NamespaceScope retval = namespace; - for (NamespaceScope namespaceScope : declaredNamespaces) { - if (!namespaceScope.isDefaultNamespace()) { - if (retval.isDefaultNamespace()) { - retval = namespaceScope; - } else if (retval.getBlockRange().getStart() > namespaceScope.getBlockRange().getStart()) { - retval = namespaceScope; - } - } - } - return retval; - } - @CheckForNull public static TypeScope getTypeScope(ModelElement element) { TypeScope retval = (element instanceof TypeScope) ? (TypeScope) element : null; diff --git a/php/php.editor/src/org/netbeans/modules/php/editor/model/impl/ModelBuilder.java b/php/php.editor/src/org/netbeans/modules/php/editor/model/impl/ModelBuilder.java index 3f3c43c466b2..7e8f33cb1f5a 100644 --- a/php/php.editor/src/org/netbeans/modules/php/editor/model/impl/ModelBuilder.java +++ b/php/php.editor/src/org/netbeans/modules/php/editor/model/impl/ModelBuilder.java @@ -86,7 +86,7 @@ class ModelBuilder { NamespaceScope build(NamespaceDeclaration node, OccurenceBuilder occurencesBuilder) { final NamespaceDeclarationInfo info = NamespaceDeclarationInfo.create(node); - NamespaceScopeImpl nScope = /*(info.isDefaultNamespace()) ? defaultNamespaceScope :*/ ModelElementFactory.create(info, this); + NamespaceScopeImpl nScope = ModelElementFactory.create(info, this); if (!nScope.isDefaultNamespace()) { setCurrentScope(nScope); } diff --git a/php/php.editor/test/unit/data/goldenfiles/org/netbeans/modules/php/editor/csl/NavigatorTest/structure/php53/bracketed_multiple_namespaces_with_multiple_default.pass b/php/php.editor/test/unit/data/goldenfiles/org/netbeans/modules/php/editor/csl/NavigatorTest/structure/php53/bracketed_multiple_namespaces_with_multiple_default.pass index 2a4046e32aa2..7e8665714640 100644 --- a/php/php.editor/test/unit/data/goldenfiles/org/netbeans/modules/php/editor/csl/NavigatorTest/structure/php53/bracketed_multiple_namespaces_with_multiple_default.pass +++ b/php/php.editor/test/unit/data/goldenfiles/org/netbeans/modules/php/editor/csl/NavigatorTest/structure/php53/bracketed_multiple_namespaces_with_multiple_default.pass @@ -1,9 +1,9 @@ -|- [7, 61] : ESCAPED{} -|--Utils [29, 37] : ESCAPED{Utils} -|--fix [51, 59] : ESCAPED{fix}ESCAPED{(}ESCAPED{)} -|-MyProject [72, 135] : ESCAPED{MyProject} -|--Connection [94, 107] : ESCAPED{Connection} -|--connect [121, 133] : ESCAPED{connect}ESCAPED{(}ESCAPED{)} -|- [136, 199] : ESCAPED{} -|--Connection [158, 171] : ESCAPED{Connection} -|--connect [185, 197] : ESCAPED{connect}ESCAPED{(}ESCAPED{)} +|- [815, 869] : ESCAPED{} +|--Utils [837, 845] : ESCAPED{Utils} +|--fix [859, 867] : ESCAPED{fix}ESCAPED{(}ESCAPED{)} +|-MyProject [880, 943] : ESCAPED{MyProject} +|--Connection [902, 915] : ESCAPED{Connection} +|--connect [929, 941] : ESCAPED{connect}ESCAPED{(}ESCAPED{)} +|- [944, 1007] : ESCAPED{} +|--Connection [966, 979] : ESCAPED{Connection} +|--connect [993, 1005] : ESCAPED{connect}ESCAPED{(}ESCAPED{)} \ No newline at end of file diff --git a/php/php.editor/test/unit/data/goldenfiles/org/netbeans/modules/php/editor/csl/NavigatorTest/structure/php53/multiple_namespaces_with_elements_from_default.pass b/php/php.editor/test/unit/data/goldenfiles/org/netbeans/modules/php/editor/csl/NavigatorTest/structure/php53/multiple_namespaces_with_elements_from_default.pass deleted file mode 100644 index 114d67cc53a0..000000000000 --- a/php/php.editor/test/unit/data/goldenfiles/org/netbeans/modules/php/editor/csl/NavigatorTest/structure/php53/multiple_namespaces_with_elements_from_default.pass +++ /dev/null @@ -1,7 +0,0 @@ -|-Connection [13, 26] : ESCAPED{Connection} -|-MyProject [37, 89] : ESCAPED{MyProject} -|--Connection [54, 67] : ESCAPED{Connection} -|--connect [77, 89] : ESCAPED{connect}ESCAPED{(}ESCAPED{)} -|-AnotherProject [100, 161] : ESCAPED{AnotherProject} -|--Connection [122, 135] : ESCAPED{Connection} -|--connect [145, 157] : ESCAPED{connect}ESCAPED{(}ESCAPED{)} \ No newline at end of file diff --git a/php/php.editor/test/unit/data/testfiles/actions/nb4978_08.php.usedNames b/php/php.editor/test/unit/data/testfiles/actions/nb4978_08.php.usedNames index 8b137891791f..2d3c27be0ded 100644 --- a/php/php.editor/test/unit/data/testfiles/actions/nb4978_08.php.usedNames +++ b/php/php.editor/test/unit/data/testfiles/actions/nb4978_08.php.usedNames @@ -1 +1,4 @@ - +Name: Foo + Foo --> Foo:1053 + Foo --> Foo:1101 + Foo --> Foo:1113 diff --git a/php/php.editor/test/unit/data/testfiles/actions/testMultipleDefaultNSUse/01/testMultipleDefaultNSUse_01.php b/php/php.editor/test/unit/data/testfiles/actions/testMultipleDefaultNSUse/01/testMultipleDefaultNSUse_01.php new file mode 100644 index 000000000000..b6575a06ba9f --- /dev/null +++ b/php/php.editor/test/unit/data/testfiles/actions/testMultipleDefaultNSUse/01/testMultipleDefaultNSUse_01.php @@ -0,0 +1,52 @@ + +?> \ No newline at end of file diff --git a/php/php.editor/test/unit/data/testfiles/structure/php53/multiple_namespaces_with_elements_from_default.php b/php/php.editor/test/unit/data/testfiles/structure/php53/multiple_namespaces_with_elements_from_default.php deleted file mode 100644 index ffa85adf37c4..000000000000 --- a/php/php.editor/test/unit/data/testfiles/structure/php53/multiple_namespaces_with_elements_from_default.php +++ /dev/null @@ -1,10 +0,0 @@ - - diff --git a/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/actions/FixUsesPerformerTest.java b/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/actions/FixUsesPerformerTest.java index 0385913a25a1..3a9bc5e0e128 100644 --- a/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/actions/FixUsesPerformerTest.java +++ b/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/actions/FixUsesPerformerTest.java @@ -136,7 +136,7 @@ public void testIssue222595_04() throws Exception { } public void testIssue238828() throws Exception { - String[] selections = new String[] {"\\First\\Second\\Util", CAN_NOT_BE_RESOLVED, CAN_NOT_BE_RESOLVED, CAN_NOT_BE_RESOLVED}; + String[] selections = new String[] {CAN_NOT_BE_RESOLVED, CAN_NOT_BE_RESOLVED, "\\First\\Second\\Util", CAN_NOT_BE_RESOLVED}; Options options = new Options(true, false, true, true, false); performTest("function functionName3($param) {}^", createSelections(selections, ItemVariant.Type.CLASS), false, options); } @@ -401,6 +401,30 @@ public void testGH5578_08() throws Exception { Options options = new Options(false, false, false, false, true); performTest("// test^", createSelections(selections, ItemVariant.Type.CLASS), true, options); } + + public void testMultipleDefaultNSUse_01() throws Exception { + String[] selections = new String[] {"\\NS2\\B"}; + Options options = new Options(false, false, false, false, true); + performTest("// test^", createSelections(selections, ItemVariant.Type.CLASS), true, options); + } + + public void testMultipleDefaultNSUse_02() throws Exception { + String[] selections = new String[] {"\\NS2\\D"}; + Options options = new Options(false, false, false, false, true); + performTest("// test^", createSelections(selections, ItemVariant.Type.CLASS), true, options); + } + + public void testWholeFileUse_01() throws Exception { + String[] selections = new String[] {"\\NS2\\B", "\\NS2\\D"}; + Options options = new Options(false, false, false, false, true); + performTest("// test^", createSelections(selections, ItemVariant.Type.CLASS), true, options); + } + + public void testWholeFileUse_02() throws Exception { + String[] selections = new String[] {"\\NS2\\B", "\\NS2\\D", "\\NS2\\F"}; + Options options = new Options(false, false, false, false, true); + performTest("// test^", createSelections(selections, ItemVariant.Type.CLASS), true, options); + } private String getTestResult(final String fileName, final String caretLine, final List selections, final boolean removeUnusedUses, final Options options) throws Exception { FileObject testFile = getTestFile(fileName); @@ -443,6 +467,9 @@ public void run(final ResultIterator resultIterator) throws Exception { namespaceScope.getNamespaceName(), currentOptions).create(); final List properSelections = new ArrayList<>(); + // this logic probably needs refactoring because selections always + // should be paired with import data items by index and in the feature it + // could be a problem if data items order will change for (Selection selection : selections) { properSelections.add(new ItemVariant( selection.getSelection(), diff --git a/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/csl/NavigatorTest.java b/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/csl/NavigatorTest.java index cb06815e71c0..8c9248a4a7c8 100644 --- a/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/csl/NavigatorTest.java +++ b/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/csl/NavigatorTest.java @@ -35,10 +35,6 @@ public void testNamespace() throws Exception { public void testMultiple_namespaces() throws Exception { performTest("structure/php53/multiple_namespaces"); } - - public void testMultiple_namespaces_with_elements_from_default() throws Exception { - performTest("structure/php53/multiple_namespaces_with_elements_from_default"); - } public void testBracketedMultipleNamespaces() throws Exception { performTest("structure/php53/bracketed_multiple_namespaces");