From 94a5f6708400041f6fc3320161138855f1a45550 Mon Sep 17 00:00:00 2001 From: Craig Sennabaum Date: Tue, 22 Nov 2022 07:10:43 -0500 Subject: [PATCH 1/5] initial approach --- .../cql/ls/server/config/ServerConfig.java | 10 +- .../provider/GoToDefinitionProvider.java | 241 ++++++++++++++++++ .../service/CqlTextDocumentService.java | 28 +- .../server/utility/OverlappingElements.java | 60 +++++ .../visitor/ExpressionOverlapVisitor.java | 24 ++ .../ExpressionOverlapVisitorContext.java | 59 +++++ .../provider/FindFunctionRefVisitor.java | 30 +++ .../provider/GoToDefinitionProviderTest.java | 83 ++++++ .../visitor/ExpressionOverlapVisitorTest.java | 51 ++++ .../cqf/cql/ls/server/gotodefinition/GoTo.cql | 27 ++ .../ls/server/gotodefinition/OtherFile.cql | 9 + pom.xml | 1 + 12 files changed, 607 insertions(+), 16 deletions(-) create mode 100644 ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java create mode 100644 ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/OverlappingElements.java create mode 100644 ls/server/src/main/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitor.java create mode 100644 ls/server/src/main/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorContext.java create mode 100644 ls/server/src/test/java/org/opencds/cqf/cql/ls/server/provider/FindFunctionRefVisitor.java create mode 100644 ls/server/src/test/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProviderTest.java create mode 100644 ls/server/src/test/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorTest.java create mode 100644 ls/server/src/test/resources/org/opencds/cqf/cql/ls/server/gotodefinition/GoTo.cql create mode 100644 ls/server/src/test/resources/org/opencds/cqf/cql/ls/server/gotodefinition/OtherFile.cql diff --git a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/config/ServerConfig.java b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/config/ServerConfig.java index 8ff2cb8..4aaeefb 100644 --- a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/config/ServerConfig.java +++ b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/config/ServerConfig.java @@ -12,6 +12,7 @@ import org.opencds.cqf.cql.ls.server.manager.CqlTranslationManager; import org.opencds.cqf.cql.ls.server.manager.TranslatorOptionsManager; import org.opencds.cqf.cql.ls.server.plugin.CommandContribution; +import org.opencds.cqf.cql.ls.server.provider.GoToDefinitionProvider; import org.opencds.cqf.cql.ls.server.provider.FormattingProvider; import org.opencds.cqf.cql.ls.server.provider.HoverProvider; import org.opencds.cqf.cql.ls.server.service.ActiveContentService; @@ -68,9 +69,9 @@ public CompletableFuture languageClient() { @Bean public CqlTextDocumentService cqlTextDocumentService( - CompletableFuture languageClient, HoverProvider hoverProvider, + CompletableFuture languageClient, HoverProvider hoverProvider, GoToDefinitionProvider goToDefinitionProvider, FormattingProvider formattingProvider, EventBus eventBus) { - return new CqlTextDocumentService(languageClient, hoverProvider, formattingProvider, + return new CqlTextDocumentService(languageClient, hoverProvider, formattingProvider, goToDefinitionProvider, eventBus); } @@ -105,6 +106,11 @@ HoverProvider hoverProvider(CqlTranslationManager cqlTranslationManager) { return new HoverProvider(cqlTranslationManager); } + @Bean + GoToDefinitionProvider definitionProvider (CqlTranslationManager cqlTranslationManager) { + return new GoToDefinitionProvider(cqlTranslationManager); + } + @Bean FormattingProvider formattingProvider(ContentService contentService) { return new FormattingProvider(contentService); diff --git a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java new file mode 100644 index 0000000..57f7165 --- /dev/null +++ b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java @@ -0,0 +1,241 @@ +package org.opencds.cqf.cql.ls.server.provider; + +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.cqframework.cql.cql2elm.CqlTranslator; +import org.cqframework.cql.elm.tracking.TrackBack; +import org.eclipse.lsp4j.*; +import org.eclipse.lsp4j.jsonrpc.messages.Either; +import org.hl7.cql.model.DataType; +import org.hl7.elm.r1.*; +import org.opencds.cqf.cql.ls.core.utility.Uris; +import org.opencds.cqf.cql.ls.server.manager.CqlTranslationManager; +import org.opencds.cqf.cql.ls.server.service.FileContentService; +import org.opencds.cqf.cql.ls.server.utility.OverlappingElements; +import org.opencds.cqf.cql.ls.server.visitor.ExpressionOverlapVisitor; +import org.opencds.cqf.cql.ls.server.visitor.ExpressionOverlapVisitorContext; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.xml.namespace.QName; +import java.io.File; +import java.net.URI; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +public class GoToDefinitionProvider { + private static final Logger log = LoggerFactory.getLogger(GoToDefinitionProvider.class); + private CqlTranslationManager cqlTranslationManager; + private ExpressionOverlapVisitor expressionOverlapVisitor = new ExpressionOverlapVisitor(); + + public GoToDefinitionProvider(CqlTranslationManager cqlTranslationManager) { + this.cqlTranslationManager = cqlTranslationManager; + } + + public Either, List> getDefinitionLocation (DefinitionParams params) { + + ArrayList locations = new ArrayList<>(); + + URI uri = Uris.parseOrNull(params.getTextDocument().getUri()); + if (uri == null) { + return Either.forRight(locations); + } + + // Cql handles position indexes different then VsCode + Position initialPosition = params.getPosition(); + Position cqlPosition = new Position(initialPosition.getLine() + 1, initialPosition.getCharacter() + 1); + + // This translates on the fly. We may want to consider maintaining + // an ELM index to reduce the need to do retranslation. + CqlTranslator translator = this.cqlTranslationManager.translate(uri); + if (translator == null) { + return Either.forRight(locations); + } + + Library library = translator.getTranslatedLibrary().getLibrary(); + ExpressionOverlapVisitorContext context = new ExpressionOverlapVisitorContext(cqlPosition); + this.expressionOverlapVisitor.visitLibrary(library, context); + + Element specificElement = OverlappingElements.getMostSpecificElement(context.getOverlappingElements()); + + LocationLink locationLink = this.getLocationFromElement(specificElement, library, uri); + if (locationLink != null) { + locations.add(locationLink); + } + + return Either.forRight(locations); + } + + protected LocationLink getLocationFromElement (Element element, Library currentLibrary, URI currentUri ) { + + // This case must come before ExpressionRef case because FunctionRef inherits from ExpressionRef + if (element instanceof FunctionRef) { + FunctionRef elFunctionRef = (FunctionRef) element; + + ImmutablePair searchLibraryPair; + + // If libraryName exists search for it, else use current. + if (elFunctionRef.getLibraryName() == null) { + searchLibraryPair = new ImmutablePair(currentUri, currentLibrary); + } else { + searchLibraryPair = findIncludedLibrary(currentLibrary, elFunctionRef.getLibraryName(), Uris.getHead(currentUri)); + } + + if (searchLibraryPair != null) { + + List expressionDefCandidates = GoToDefinitionProvider.getExpressionDefCandidatesByName(searchLibraryPair.getRight(), elFunctionRef.getName()); + + // If exactly one match by name, don't worry about arguments matching + if (expressionDefCandidates.size() == 1) { + return GoToDefinitionProvider.getLocationLinkForExpressionDef(expressionDefCandidates.get(0), searchLibraryPair.getLeft(), elFunctionRef); + } + + // If more than one match, try to match calling function args with definition func args + else if (expressionDefCandidates.size() > 1) { + FunctionDef functionDefCandidate = GoToDefinitionProvider.findMatchingFunctionDefInLibrary(expressionDefCandidates, elFunctionRef); + if (functionDefCandidate != null) { + return GoToDefinitionProvider.getLocationLinkForExpressionDef(functionDefCandidate, searchLibraryPair.getLeft(), elFunctionRef); + } + } + } + } + + else if (element instanceof ExpressionRef) { + ExpressionRef elExpressionRef = (ExpressionRef) element; + + ImmutablePair searchLibraryPair; + if (elExpressionRef.getLibraryName() == null) { + searchLibraryPair = new ImmutablePair(currentUri, currentLibrary); + } else { + searchLibraryPair = findIncludedLibrary(currentLibrary, elExpressionRef.getLibraryName(), Uris.getHead(currentUri)); + } + + if (searchLibraryPair != null) { + + List expressionDefCandidates = GoToDefinitionProvider.getExpressionDefCandidatesByName(searchLibraryPair.getRight(), elExpressionRef.getName()); + + // Should be one exact match for ExpressionRef if it exists + if (expressionDefCandidates.size() == 1) { + return GoToDefinitionProvider.getLocationLinkForExpressionDef(expressionDefCandidates.get(0), searchLibraryPair.getLeft(), elExpressionRef); + } + } + } + + return null; + } + + protected static List getExpressionDefCandidatesByName (Library library, String searchName) { + return library.getStatements().getDef().stream().filter(expressionDef -> expressionDef.getName().equals(searchName)).collect(Collectors.toList()); + } + + protected static LocationLink getLocationLinkForExpressionDef (ExpressionDef foundExpressionDef, URI libraryUri, Element selectionElement) { + Range targetRange = GoToDefinitionProvider.getRangeOfElement(foundExpressionDef); + Range originSelectionRange = GoToDefinitionProvider.getRangeOfElement(selectionElement); + return new LocationLink(libraryUri.toString(), targetRange, targetRange, originSelectionRange); + } + + protected static FunctionDef findMatchingFunctionDefInLibrary (List expressionDefCandidates, FunctionRef elFunctionRef) { + + for (ExpressionDef expressionDef: expressionDefCandidates) { + if (expressionDef instanceof FunctionDef) { + FunctionDef functionDefCandidate = (FunctionDef) expressionDef; + + // Should already be filtered by name but doesnt hurt + if (!elFunctionRef.getName().equals(functionDefCandidate.getName())) { + continue; + } + + // Must have same number of arguments + if (elFunctionRef.getOperand().size() != functionDefCandidate.getOperand().size()) { + continue; + } + + boolean doesMatch = true; + + for (int i = 0; i < elFunctionRef.getOperand().size(); i++) { + OperandDef operandDefArg = functionDefCandidate.getOperand().get(i); + Expression operandRefArg = elFunctionRef.getOperand().get(i); + + doesMatch = GoToDefinitionProvider.doesFunctionRefArg(operandDefArg, operandRefArg); + + if (!doesMatch) { + break; + } + } + + if (doesMatch) { + return functionDefCandidate; + } + } + } + + return null; + } + + protected ImmutablePair findIncludedLibrary (Library searchLibrary, String libraryName, URI cqlDirectory ) { + File foundFile = null; + for (IncludeDef includeDef: searchLibrary.getIncludes().getDef()) { + if (includeDef.getLocalIdentifier().equals(libraryName)) { + VersionedIdentifier includeIdentifier = new VersionedIdentifier() + .withId(includeDef.getPath()) + .withVersion(includeDef.getVersion()); + + foundFile = FileContentService.searchFolder(cqlDirectory, includeIdentifier); + break; + } + } + + if (foundFile == null) { + return null; + } + + CqlTranslator translator = this.cqlTranslationManager.translate(foundFile.toURI()); + if (translator == null) { + return null; + } + + return new ImmutablePair(foundFile.toURI(), translator.getTranslatedLibrary().getLibrary()); + + } + + public static boolean doesFunctionRefArg(OperandDef functionDefArg, Expression functionRefArg) { + DataType functionDefArgType = functionDefArg.getOperandTypeSpecifier().getResultType(); + DataType functionRefArgType = functionRefArg.getResultType(); + return functionDefArgType.isCompatibleWith(functionRefArgType); + } + +// public static boolean compareTypeSpecifier (TypeSpecifier a, TypeSpecifier b) { +// if (a == null || b == null || a.getClass() != b.getClass()) { +// return false; +// } +// +// if (a instanceof NamedTypeSpecifier) { +// QName aName = ((NamedTypeSpecifier) a).getName(); +// QName bName = ((NamedTypeSpecifier) b).getName(); +// return aName.getLocalPart().equals(bName.getLocalPart()) && aName.getNamespaceURI().equals(bName.getNamespaceURI()) && aName.getPrefix().equals(bName.getPrefix()); +// } else if (a instanceof ListTypeSpecifier) { +// ListTypeSpecifier aList = (ListTypeSpecifier) a; +// ListTypeSpecifier bList = (ListTypeSpecifier) b; +// return GoToDefinitionProvider.compareTypeSpecifier(aList.getElementType(), bList.getElementType()); +// } else if (a instanceof IntervalTypeSpecifier) { +// IntervalTypeSpecifier aInterval = (IntervalTypeSpecifier) a; +// IntervalTypeSpecifier bInterval = (IntervalTypeSpecifier) b; +// return GoToDefinitionProvider.compareTypeSpecifier(aInterval.getPointType(), bInterval.getPointType()); +// } +// +// return false; +// } + + + public static Range getRangeOfElement (Element element) { + if (element.getTrackbacks().size() == 0) { + return null; + } + TrackBack trackBack = element.getTrackbacks().get(0); + Position startPosition = new Position(trackBack.getStartLine() - 1, trackBack.getStartChar() - 1); + Position endPosition = new Position(trackBack.getEndLine() - 1, trackBack.getEndChar()); + return new Range(startPosition, endPosition); + } + +} diff --git a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/service/CqlTextDocumentService.java b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/service/CqlTextDocumentService.java index a7b3980..a2e5d3a 100644 --- a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/service/CqlTextDocumentService.java +++ b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/service/CqlTextDocumentService.java @@ -2,19 +2,9 @@ import java.util.List; import java.util.concurrent.CompletableFuture; -import org.eclipse.lsp4j.DidChangeTextDocumentParams; -import org.eclipse.lsp4j.DidCloseTextDocumentParams; -import org.eclipse.lsp4j.DidOpenTextDocumentParams; -import org.eclipse.lsp4j.DidSaveTextDocumentParams; -import org.eclipse.lsp4j.DocumentFormattingParams; -import org.eclipse.lsp4j.Hover; -import org.eclipse.lsp4j.HoverParams; -import org.eclipse.lsp4j.InitializeParams; -import org.eclipse.lsp4j.MessageParams; -import org.eclipse.lsp4j.MessageType; -import org.eclipse.lsp4j.ServerCapabilities; -import org.eclipse.lsp4j.TextDocumentSyncKind; -import org.eclipse.lsp4j.TextEdit; + +import org.eclipse.lsp4j.*; +import org.eclipse.lsp4j.jsonrpc.messages.Either; import org.eclipse.lsp4j.services.LanguageClient; import org.eclipse.lsp4j.services.TextDocumentService; import org.greenrobot.eventbus.EventBus; @@ -22,6 +12,7 @@ import org.opencds.cqf.cql.ls.server.event.DidCloseTextDocumentEvent; import org.opencds.cqf.cql.ls.server.event.DidOpenTextDocumentEvent; import org.opencds.cqf.cql.ls.server.event.DidSaveTextDocumentEvent; +import org.opencds.cqf.cql.ls.server.provider.GoToDefinitionProvider; import org.opencds.cqf.cql.ls.server.provider.FormattingProvider; import org.opencds.cqf.cql.ls.server.provider.HoverProvider; import org.slf4j.Logger; @@ -34,13 +25,15 @@ public class CqlTextDocumentService implements TextDocumentService { private final CompletableFuture client; private final FormattingProvider formattingProvider; private final HoverProvider hoverProvider; + private final GoToDefinitionProvider goToDefinitionProvider; private final EventBus eventBus; public CqlTextDocumentService(CompletableFuture client, - HoverProvider hoverProvider, FormattingProvider formattingProvider, EventBus eventBus) { + HoverProvider hoverProvider, FormattingProvider formattingProvider, GoToDefinitionProvider goToDefinitionProvider, EventBus eventBus) { this.client = client; this.formattingProvider = formattingProvider; this.hoverProvider = hoverProvider; + this.goToDefinitionProvider = goToDefinitionProvider; this.eventBus = eventBus; } @@ -52,6 +45,8 @@ public void initialize(InitializeParams params, ServerCapabilities serverCapabil serverCapabilities.setDocumentFormattingProvider(true); // serverCapabilities.setDocumentRangeFormattingProvider(false); serverCapabilities.setHoverProvider(true); + serverCapabilities.setDefinitionProvider(true); +// serverCapabilities.set // c.setReferencesProvider(true); // c.setDocumentSymbolProvider(true); // c.setCodeActionProvider(true); @@ -65,6 +60,11 @@ public CompletableFuture hover(HoverParams position) { .exceptionally(this::notifyClient); } + @Override + public CompletableFuture, List>> definition(DefinitionParams params) { + return CompletableFuture.supplyAsync(() -> this.goToDefinitionProvider.getDefinitionLocation(params)) + .exceptionally(this::notifyClient); + } @Override public CompletableFuture> formatting(DocumentFormattingParams params) { diff --git a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/OverlappingElements.java b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/OverlappingElements.java new file mode 100644 index 0000000..ad46c6d --- /dev/null +++ b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/OverlappingElements.java @@ -0,0 +1,60 @@ +package org.opencds.cqf.cql.ls.server.utility; + +import org.cqframework.cql.elm.tracking.TrackBack; +import org.hl7.elm.r1.Element; + +import java.util.Collections; +import java.util.Comparator; +import java.util.List; + +public class OverlappingElements { + + public static Element getMostSpecificElement(List elements) { + if (elements.size() == 0) { + return null; + } + + Comparator compareElements = (o1, o2) -> { + if (o1.getTrackbacks().size() == 0) { + return 1; + } + + if (o2.getTrackbacks().size() == 0) { + return -1; + } + + TrackBack trackBackOne = o1.getTrackbacks().get(0); + TrackBack trackBackTwo = o2.getTrackbacks().get(0); + + if (trackBackOne.getStartLine() == trackBackTwo.getStartLine() + && trackBackOne.getStartChar() == trackBackTwo.getStartChar() + && trackBackOne.getEndLine() == trackBackTwo.getEndLine() + && trackBackOne.getEndChar() == trackBackTwo.getEndChar() + ) { + return 0; + } + + if (trackBackOne.getStartLine() < trackBackTwo.getStartLine()) { + return 1; + } + + if (trackBackOne.getStartLine() == trackBackTwo.getStartLine() && trackBackOne.getStartChar() < trackBackTwo.getStartChar()) { + return 1; + } + + if (trackBackOne.getEndLine() > trackBackTwo.getEndLine()) { + return 1; + } + + if (trackBackOne.getEndLine() == trackBackTwo.getEndLine() && trackBackOne.getEndChar() > trackBackTwo.getEndChar()) { + return 1; + } + + return -1; + }; + + Collections.sort(elements, compareElements); + + return elements.get(0); + } +} diff --git a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitor.java b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitor.java new file mode 100644 index 0000000..b931e6f --- /dev/null +++ b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitor.java @@ -0,0 +1,24 @@ +package org.opencds.cqf.cql.ls.server.visitor; + +import org.cqframework.cql.elm.tracking.TrackBack; +import org.cqframework.cql.elm.tracking.Trackable; +import org.cqframework.cql.elm.visiting.ElmBaseLibraryVisitor; +import org.hl7.elm.r1.Element; + +import java.util.List; + +public class ExpressionOverlapVisitor extends ElmBaseLibraryVisitor { + + @Override + protected Void defaultResult(Trackable elm, ExpressionOverlapVisitorContext context) { + if (elm instanceof Element) { + Element element = (Element) elm; + if (context.doesElementOverlapSearchPosition(element)) { + context.addOverlappingElement(element); + } + } + + return super.defaultResult(elm, context); + } + +} diff --git a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorContext.java b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorContext.java new file mode 100644 index 0000000..3ab92d2 --- /dev/null +++ b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorContext.java @@ -0,0 +1,59 @@ +package org.opencds.cqf.cql.ls.server.visitor; + +import org.cqframework.cql.elm.tracking.TrackBack; +import org.eclipse.lsp4j.Position; +import org.hl7.elm.r1.Element; + +import java.util.ArrayList; +import java.util.List; + +public class ExpressionOverlapVisitorContext { + private List overlappingElements = new ArrayList<>(); + + private Position searchPosition; + + public ExpressionOverlapVisitorContext(Position searchPosition) { + this.searchPosition = searchPosition; + } + + public List getOverlappingElements () { + return this.overlappingElements; + } + + public void addOverlappingElement (Element el) { + this.overlappingElements.add(el); + } + + protected boolean doesElementOverlapSearchPosition(Element elm) { + for (TrackBack tb : elm.getTrackbacks()) { + if (this.doesTrackBackOverlapSearchPosition(tb)) { + return true; + } + } + return false; + } + + protected boolean doesTrackBackOverlapSearchPosition (TrackBack trackback) { + // If search line is before trackback start return false + if (this.searchPosition.getLine() < trackback.getStartLine()) { + return false; + } + + // If search on same line as trackback start, search character must come on or after elementRange start + if (this.searchPosition.getLine() == trackback.getStartLine() && this.searchPosition.getCharacter() < trackback.getStartChar()) { + return false; + } + + // If search line is after trackback end return false + if (this.searchPosition.getLine() > trackback.getEndLine()) { + return false; + } + + // If search on same line as trackback end, search character must come on or after elementRange start + if (this.searchPosition.getLine() == trackback.getEndLine() && this.searchPosition.getCharacter() > trackback.getEndChar()) { + return false; + } + + return true; + } +} diff --git a/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/provider/FindFunctionRefVisitor.java b/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/provider/FindFunctionRefVisitor.java new file mode 100644 index 0000000..70ee182 --- /dev/null +++ b/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/provider/FindFunctionRefVisitor.java @@ -0,0 +1,30 @@ +package org.opencds.cqf.cql.ls.server.provider; +import org.cqframework.cql.elm.tracking.Trackable; +import org.cqframework.cql.elm.visiting.ElmBaseLibraryVisitor; +import org.hl7.elm.r1.FunctionRef; + +public class FindFunctionRefVisitor extends ElmBaseLibraryVisitor { + + @Override + protected FunctionRef aggregateResult(FunctionRef aggregate, FunctionRef nextResult) { + if (nextResult != null) { + return nextResult; + } + if (aggregate != null) { + return aggregate; + } + return super.aggregateResult(aggregate, nextResult); + } + + @Override + protected FunctionRef defaultResult(Trackable elm, String searchName) { + if (elm instanceof FunctionRef) { + FunctionRef functionRef = (FunctionRef) elm; + if (functionRef.getName().equals(searchName)) { + return functionRef; + } + } + + return super.defaultResult(elm, searchName); + } +} diff --git a/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProviderTest.java b/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProviderTest.java new file mode 100644 index 0000000..1ee0620 --- /dev/null +++ b/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProviderTest.java @@ -0,0 +1,83 @@ +package org.opencds.cqf.cql.ls.server.provider; + +import org.hl7.elm.r1.FunctionDef; +import org.hl7.elm.r1.FunctionRef; +import org.hl7.elm.r1.Library; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.opencds.cqf.cql.ls.core.ContentService; +import org.opencds.cqf.cql.ls.core.utility.Uris; +import org.opencds.cqf.cql.ls.server.manager.CqlTranslationManager; +import org.opencds.cqf.cql.ls.server.manager.TranslatorOptionsManager; +import org.opencds.cqf.cql.ls.server.service.TestContentService; + +import static org.junit.jupiter.api.Assertions.*; + +public class GoToDefinitionProviderTest { + + private static Library goToLibrary; + private static Library otherFileLibrary; + + private static FindFunctionRefVisitor findFunctionRefVisitor = new FindFunctionRefVisitor(); + + @BeforeAll + public static void beforeAll() { + ContentService cs = new TestContentService(); + CqlTranslationManager cqlTranslationManager = + new CqlTranslationManager(cs, new TranslatorOptionsManager(cs)); + + goToLibrary = cqlTranslationManager.translate(Uris.parseOrNull( + "/org/opencds/cqf/cql/ls/server/gotodefinition/GoTo.cql")) + .getTranslatedLibrary().getLibrary(); + + otherFileLibrary = cqlTranslationManager.translate(Uris.parseOrNull( + "/org/opencds/cqf/cql/ls/server/gotodefinition/OtherFile.cql")) + .getTranslatedLibrary().getLibrary(); + } + +// @Test +// public void hoverInt() throws Exception { +// +// assertNotNull(library); + +// assertEquals(2, 3); +// Hover hover = hoverProvider.hover(new HoverParams( +// new TextDocumentIdentifier("/org/opencds/cqf/cql/ls/server/Two.cql"), +// new Position(5, 2))); +// +// assertNotNull(hover); +// assertNotNull(hover.getContents().getRight()); +// +// MarkupContent markup = hover.getContents().getRight(); +// assertEquals("markdown", markup.getKind()); +// assertEquals("```cql\nSystem.Integer\n```", markup.getValue()); +// } + + + @Test void findMatchingFunctionRefForInt () { + FunctionRef foundRef = findFunctionRefVisitor.visitLibrary(goToLibrary, "Int Func"); + assertNotNull(foundRef); + + FunctionDef foundDef = GoToDefinitionProvider.findMatchingFunctionDefInLibrary(goToLibrary.getStatements().getDef(), foundRef); + assertNotNull(foundDef); + } + + @Test void findMatchingFunctionRefForValueSet () { + FunctionRef foundRef = findFunctionRefVisitor.visitLibrary(goToLibrary, "VS Func"); + assertNotNull(foundRef); + + FunctionDef foundDef = GoToDefinitionProvider.findMatchingFunctionDefInLibrary(goToLibrary.getStatements().getDef(), foundRef); + assertNotNull(foundDef); + } + + + @Test void findMatchingFunctionWithDateTimeCast () { + FunctionRef foundRef = findFunctionRefVisitor.visitLibrary(goToLibrary, "With Date Time"); + assertNotNull(foundRef); + + FunctionDef foundDef = GoToDefinitionProvider.findMatchingFunctionDefInLibrary(goToLibrary.getStatements().getDef(), foundRef); + assertNotNull(foundDef); + } + +} + diff --git a/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorTest.java b/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorTest.java new file mode 100644 index 0000000..2a74f59 --- /dev/null +++ b/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorTest.java @@ -0,0 +1,51 @@ +package org.opencds.cqf.cql.ls.server.visitor; + +import org.eclipse.lsp4j.Position; +import org.hl7.elm.r1.Element; +import org.hl7.elm.r1.ExpressionRef; +import org.hl7.elm.r1.Library; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.opencds.cqf.cql.ls.core.ContentService; +import org.opencds.cqf.cql.ls.core.utility.Uris; +import org.opencds.cqf.cql.ls.server.manager.CqlTranslationManager; +import org.opencds.cqf.cql.ls.server.manager.TranslatorOptionsManager; +import org.opencds.cqf.cql.ls.server.service.TestContentService; +import org.opencds.cqf.cql.ls.server.utility.OverlappingElements; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; + +public class ExpressionOverlapVisitorTest { + private static Library library; + + @BeforeAll + public static void beforeAll() { + ContentService cs = new TestContentService(); + CqlTranslationManager cqlTranslationManager = + new CqlTranslationManager(cs, new TranslatorOptionsManager(cs)); + library = cqlTranslationManager.translate(Uris.parseOrNull( + "/org/opencds/cqf/cql/ls/server/gotodefinition/GoTo.cql")) + .getTranslatedLibrary().getLibrary(); + } + + @Test + public void shouldGetExpressionRefAtPos() throws Exception { + + ExpressionOverlapVisitor expressionOverlapVisitor = new ExpressionOverlapVisitor(); + Position position = new Position(14, 28); + + ExpressionOverlapVisitorContext context = new ExpressionOverlapVisitorContext(position); + expressionOverlapVisitor.visitLibrary(library, context); + + List overlappingElements = context.getOverlappingElements(); + assertEquals(3, overlappingElements.size()); + + Element specificElement = OverlappingElements.getMostSpecificElement(overlappingElements); + assertTrue(specificElement instanceof ExpressionRef); + + ExpressionRef el = (ExpressionRef) specificElement; + assertTrue(el.getName().equals("Add Def")); + } +} diff --git a/ls/server/src/test/resources/org/opencds/cqf/cql/ls/server/gotodefinition/GoTo.cql b/ls/server/src/test/resources/org/opencds/cqf/cql/ls/server/gotodefinition/GoTo.cql new file mode 100644 index 0000000..9df8692 --- /dev/null +++ b/ls/server/src/test/resources/org/opencds/cqf/cql/ls/server/gotodefinition/GoTo.cql @@ -0,0 +1,27 @@ +library GoTo version '0.0.1' + +using FHIR version '4.0.1' + +include FHIRHelpers version '4.0.1' +include OtherFile version '0.0.1' + +valueset "Acute Inpatient": 'https://company.org/fhir/valueset/2.16.840.1.113883.3.464.1004.1810' + +context Patient + +define "Add Def": 1 + +define "Go To Local": 2 + "Add Def" + +define "Go To External": 2 + OtherFile."Another Def" + +define doInt: "Int Func"(4) +define doVS: "VS Func"("Acute Inpatient") +define doConditions: "Condition Func"([Condition]) +define doDateTime: [Observation] Obs + sort by "With Date Time"(effective as FHIR.dateTime) + +define function "Int Func" (n System.Integer): 3 + n +define function "VS Func" (vs System.ValueSet): Count(vs) +define function "Condition Func" (conds List): Count(conds) +define function "With Date Time"(value dateTime): value.value diff --git a/ls/server/src/test/resources/org/opencds/cqf/cql/ls/server/gotodefinition/OtherFile.cql b/ls/server/src/test/resources/org/opencds/cqf/cql/ls/server/gotodefinition/OtherFile.cql new file mode 100644 index 0000000..2c7856c --- /dev/null +++ b/ls/server/src/test/resources/org/opencds/cqf/cql/ls/server/gotodefinition/OtherFile.cql @@ -0,0 +1,9 @@ +library OtherFile version '0.0.1' + +using FHIR version '4.0.1' + +include FHIRHelpers version '4.0.1' + +context Patient + +define "Another Def": 1 \ No newline at end of file diff --git a/pom.xml b/pom.xml index 4abb429..16063cf 100644 --- a/pom.xml +++ b/pom.xml @@ -279,6 +279,7 @@ maven-javadoc-plugin 3.2.0 + ${java.home}/bin/javadoc -Xdoclint:all -Xdoclint:-missing -Xdoclint:all -Xdoclint:-missing From c65f5ce7077bbbda07c3b2c7b502d539ea5b993b Mon Sep 17 00:00:00 2001 From: Craig Sennabaum Date: Tue, 22 Nov 2022 07:45:58 -0500 Subject: [PATCH 2/5] a little cleanup --- .../provider/GoToDefinitionProvider.java | 98 ++++++++++--------- .../server/utility/OverlappingElements.java | 3 + 2 files changed, 57 insertions(+), 44 deletions(-) diff --git a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java index 57f7165..f8e3eff 100644 --- a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java +++ b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java @@ -1,7 +1,6 @@ package org.opencds.cqf.cql.ls.server.provider; import org.apache.commons.lang3.tuple.ImmutablePair; -import org.apache.commons.lang3.tuple.Pair; import org.cqframework.cql.cql2elm.CqlTranslator; import org.cqframework.cql.elm.tracking.TrackBack; import org.eclipse.lsp4j.*; @@ -17,7 +16,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.xml.namespace.QName; import java.io.File; import java.net.URI; import java.util.ArrayList; @@ -33,7 +31,7 @@ public GoToDefinitionProvider(CqlTranslationManager cqlTranslationManager) { this.cqlTranslationManager = cqlTranslationManager; } - public Either, List> getDefinitionLocation (DefinitionParams params) { + public Either, List> getDefinitionLocation(DefinitionParams params) { ArrayList locations = new ArrayList<>(); @@ -59,7 +57,7 @@ public Either, List> getDefinit Element specificElement = OverlappingElements.getMostSpecificElement(context.getOverlappingElements()); - LocationLink locationLink = this.getLocationFromElement(specificElement, library, uri); + LocationLink locationLink = this.getLocationLinkOfElement(specificElement, library, uri); if (locationLink != null) { locations.add(locationLink); } @@ -67,32 +65,64 @@ public Either, List> getDefinit return Either.forRight(locations); } - protected LocationLink getLocationFromElement (Element element, Library currentLibrary, URI currentUri ) { + protected LocationLink getLocationLinkOfElement(Element element, Library currentLibrary, URI currentUri) { // This case must come before ExpressionRef case because FunctionRef inherits from ExpressionRef - if (element instanceof FunctionRef) { - FunctionRef elFunctionRef = (FunctionRef) element; +// if (element instanceof FunctionRef) { +// FunctionRef elFunctionRef = (FunctionRef) element; +// +// ImmutablePair searchLibraryPair; +// +// // If libraryName exists search for it, else use current. +// if (elFunctionRef.getLibraryName() == null) { +// searchLibraryPair = new ImmutablePair(currentUri, currentLibrary); +// } else { +// searchLibraryPair = findIncludedLibrary(currentLibrary, elFunctionRef.getLibraryName(), Uris.getHead(currentUri)); +// } +// +// if (searchLibraryPair != null) { +// +// List expressionDefCandidates = GoToDefinitionProvider.getExpressionDefCandidatesByName(searchLibraryPair.getRight(), elFunctionRef.getName()); +// +// // If exactly one match by name, don't worry about arguments matching +// if (expressionDefCandidates.size() == 1) { +// return GoToDefinitionProvider.getLocationLinkForExpressionDef(expressionDefCandidates.get(0), searchLibraryPair.getLeft(), elFunctionRef); +// } +// +// // If more than one match, try to match calling function args with definition func args +// else if (expressionDefCandidates.size() > 1) { +// FunctionDef functionDefCandidate = GoToDefinitionProvider.findMatchingFunctionDefInLibrary(expressionDefCandidates, elFunctionRef); +// if (functionDefCandidate != null) { +// return GoToDefinitionProvider.getLocationLinkForExpressionDef(functionDefCandidate, searchLibraryPair.getLeft(), elFunctionRef); +// } +// } +// } +// } + + if (element instanceof ExpressionRef) { + ExpressionRef elExpressionRef = (ExpressionRef) element; ImmutablePair searchLibraryPair; // If libraryName exists search for it, else use current. - if (elFunctionRef.getLibraryName() == null) { + if (elExpressionRef.getLibraryName() == null) { searchLibraryPair = new ImmutablePair(currentUri, currentLibrary); } else { - searchLibraryPair = findIncludedLibrary(currentLibrary, elFunctionRef.getLibraryName(), Uris.getHead(currentUri)); + searchLibraryPair = findIncludedLibrary(currentLibrary, elExpressionRef.getLibraryName(), Uris.getHead(currentUri)); } if (searchLibraryPair != null) { - List expressionDefCandidates = GoToDefinitionProvider.getExpressionDefCandidatesByName(searchLibraryPair.getRight(), elFunctionRef.getName()); + List expressionDefCandidates = GoToDefinitionProvider.getExpressionDefCandidatesByName(searchLibraryPair.getRight(), elExpressionRef.getName()); - // If exactly one match by name, don't worry about arguments matching + // If one exact match for either ExpressionRef or FunctionRef, go to it if (expressionDefCandidates.size() == 1) { - return GoToDefinitionProvider.getLocationLinkForExpressionDef(expressionDefCandidates.get(0), searchLibraryPair.getLeft(), elFunctionRef); + return GoToDefinitionProvider.getLocationLinkForExpressionDef(expressionDefCandidates.get(0), searchLibraryPair.getLeft(), elExpressionRef); } - // If more than one match, try to match calling function args with definition func args - else if (expressionDefCandidates.size() > 1) { + // If more than one match for FunctionRef only, search for the best match among overloads based on arguments + else if (expressionDefCandidates.size() > 1 && element instanceof FunctionRef) { + FunctionRef elFunctionRef = (FunctionRef) element; FunctionDef functionDefCandidate = GoToDefinitionProvider.findMatchingFunctionDefInLibrary(expressionDefCandidates, elFunctionRef); if (functionDefCandidate != null) { return GoToDefinitionProvider.getLocationLinkForExpressionDef(functionDefCandidate, searchLibraryPair.getLeft(), elFunctionRef); @@ -101,43 +131,22 @@ else if (expressionDefCandidates.size() > 1) { } } - else if (element instanceof ExpressionRef) { - ExpressionRef elExpressionRef = (ExpressionRef) element; - - ImmutablePair searchLibraryPair; - if (elExpressionRef.getLibraryName() == null) { - searchLibraryPair = new ImmutablePair(currentUri, currentLibrary); - } else { - searchLibraryPair = findIncludedLibrary(currentLibrary, elExpressionRef.getLibraryName(), Uris.getHead(currentUri)); - } - - if (searchLibraryPair != null) { - - List expressionDefCandidates = GoToDefinitionProvider.getExpressionDefCandidatesByName(searchLibraryPair.getRight(), elExpressionRef.getName()); - - // Should be one exact match for ExpressionRef if it exists - if (expressionDefCandidates.size() == 1) { - return GoToDefinitionProvider.getLocationLinkForExpressionDef(expressionDefCandidates.get(0), searchLibraryPair.getLeft(), elExpressionRef); - } - } - } - return null; } - protected static List getExpressionDefCandidatesByName (Library library, String searchName) { + protected static List getExpressionDefCandidatesByName(Library library, String searchName) { return library.getStatements().getDef().stream().filter(expressionDef -> expressionDef.getName().equals(searchName)).collect(Collectors.toList()); } - protected static LocationLink getLocationLinkForExpressionDef (ExpressionDef foundExpressionDef, URI libraryUri, Element selectionElement) { + protected static LocationLink getLocationLinkForExpressionDef(ExpressionDef foundExpressionDef, URI libraryUri, Element selectionElement) { Range targetRange = GoToDefinitionProvider.getRangeOfElement(foundExpressionDef); Range originSelectionRange = GoToDefinitionProvider.getRangeOfElement(selectionElement); return new LocationLink(libraryUri.toString(), targetRange, targetRange, originSelectionRange); } - protected static FunctionDef findMatchingFunctionDefInLibrary (List expressionDefCandidates, FunctionRef elFunctionRef) { + protected static FunctionDef findMatchingFunctionDefInLibrary(List expressionDefCandidates, FunctionRef elFunctionRef) { - for (ExpressionDef expressionDef: expressionDefCandidates) { + for (ExpressionDef expressionDef : expressionDefCandidates) { if (expressionDef instanceof FunctionDef) { FunctionDef functionDefCandidate = (FunctionDef) expressionDef; @@ -157,7 +166,7 @@ protected static FunctionDef findMatchingFunctionDefInLibrary (List findIncludedLibrary (Library searchLibrary, String libraryName, URI cqlDirectory ) { + protected ImmutablePair findIncludedLibrary(Library searchLibrary, String libraryName, URI cqlDirectory) { File foundFile = null; - for (IncludeDef includeDef: searchLibrary.getIncludes().getDef()) { + for (IncludeDef includeDef : searchLibrary.getIncludes().getDef()) { + // Use LocalIdentifier for lookup if (includeDef.getLocalIdentifier().equals(libraryName)) { VersionedIdentifier includeIdentifier = new VersionedIdentifier() .withId(includeDef.getPath()) @@ -199,7 +209,7 @@ protected ImmutablePair findIncludedLibrary (Library searchLibrary } - public static boolean doesFunctionRefArg(OperandDef functionDefArg, Expression functionRefArg) { + public static boolean areFunctionArgsCompatible(OperandDef functionDefArg, Expression functionRefArg) { DataType functionDefArgType = functionDefArg.getOperandTypeSpecifier().getResultType(); DataType functionRefArgType = functionRefArg.getResultType(); return functionDefArgType.isCompatibleWith(functionRefArgType); @@ -228,7 +238,7 @@ public static boolean doesFunctionRefArg(OperandDef functionDefArg, Expression f // } - public static Range getRangeOfElement (Element element) { + public static Range getRangeOfElement(Element element) { if (element.getTrackbacks().size() == 0) { return null; } diff --git a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/OverlappingElements.java b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/OverlappingElements.java index ad46c6d..46328c6 100644 --- a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/OverlappingElements.java +++ b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/OverlappingElements.java @@ -7,6 +7,9 @@ import java.util.Comparator; import java.util.List; +// Here we want to find the most specific element in a list of elements that match +// a particular cursor position. So after collecting all elements that overlap, +// we sort them from smallest to largest and take the smallest matching element. public class OverlappingElements { public static Element getMostSpecificElement(List elements) { From 3fa6ad122177eba9977024e2993d2620d6d51163 Mon Sep 17 00:00:00 2001 From: Craig Sennabaum Date: Wed, 23 Nov 2022 07:15:56 -0500 Subject: [PATCH 3/5] make overlapping elements class easier --- .../provider/GoToDefinitionProvider.java | 9 +++------ ...nts.java => CursorOverlappingElements.java} | 18 +++++++++++++++++- .../visitor/ExpressionOverlapVisitorTest.java | 11 +++-------- 3 files changed, 23 insertions(+), 15 deletions(-) rename ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/{OverlappingElements.java => CursorOverlappingElements.java} (68%) diff --git a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java index f8e3eff..03d47fe 100644 --- a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java +++ b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java @@ -10,7 +10,7 @@ import org.opencds.cqf.cql.ls.core.utility.Uris; import org.opencds.cqf.cql.ls.server.manager.CqlTranslationManager; import org.opencds.cqf.cql.ls.server.service.FileContentService; -import org.opencds.cqf.cql.ls.server.utility.OverlappingElements; +import org.opencds.cqf.cql.ls.server.utility.CursorOverlappingElements; import org.opencds.cqf.cql.ls.server.visitor.ExpressionOverlapVisitor; import org.opencds.cqf.cql.ls.server.visitor.ExpressionOverlapVisitorContext; import org.slf4j.Logger; @@ -25,7 +25,7 @@ public class GoToDefinitionProvider { private static final Logger log = LoggerFactory.getLogger(GoToDefinitionProvider.class); private CqlTranslationManager cqlTranslationManager; - private ExpressionOverlapVisitor expressionOverlapVisitor = new ExpressionOverlapVisitor(); + public GoToDefinitionProvider(CqlTranslationManager cqlTranslationManager) { this.cqlTranslationManager = cqlTranslationManager; @@ -52,10 +52,7 @@ public Either, List> getDefinit } Library library = translator.getTranslatedLibrary().getLibrary(); - ExpressionOverlapVisitorContext context = new ExpressionOverlapVisitorContext(cqlPosition); - this.expressionOverlapVisitor.visitLibrary(library, context); - - Element specificElement = OverlappingElements.getMostSpecificElement(context.getOverlappingElements()); + Element specificElement = CursorOverlappingElements.getMostSpecificElementAtPosition(cqlPosition, library); LocationLink locationLink = this.getLocationLinkOfElement(specificElement, library, uri); if (locationLink != null) { diff --git a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/OverlappingElements.java b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/CursorOverlappingElements.java similarity index 68% rename from ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/OverlappingElements.java rename to ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/CursorOverlappingElements.java index 46328c6..d1b97b3 100644 --- a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/OverlappingElements.java +++ b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/CursorOverlappingElements.java @@ -1,7 +1,11 @@ package org.opencds.cqf.cql.ls.server.utility; import org.cqframework.cql.elm.tracking.TrackBack; +import org.eclipse.lsp4j.Position; import org.hl7.elm.r1.Element; +import org.hl7.elm.r1.Library; +import org.opencds.cqf.cql.ls.server.visitor.ExpressionOverlapVisitor; +import org.opencds.cqf.cql.ls.server.visitor.ExpressionOverlapVisitorContext; import java.util.Collections; import java.util.Comparator; @@ -10,7 +14,19 @@ // Here we want to find the most specific element in a list of elements that match // a particular cursor position. So after collecting all elements that overlap, // we sort them from smallest to largest and take the smallest matching element. -public class OverlappingElements { +public class CursorOverlappingElements { + + public static Element getMostSpecificElementAtPosition (Position searchPosition, Library library) { + List elements = CursorOverlappingElements.getElementsAtPosition(searchPosition, library); + return CursorOverlappingElements.getMostSpecificElement(elements); + } + + public static List getElementsAtPosition (Position searchPosition, Library library) { + ExpressionOverlapVisitor expressionOverlapVisitor = new ExpressionOverlapVisitor(); + ExpressionOverlapVisitorContext context = new ExpressionOverlapVisitorContext(searchPosition); + expressionOverlapVisitor.visitLibrary(library, context); + return context.getOverlappingElements(); + } public static Element getMostSpecificElement(List elements) { if (elements.size() == 0) { diff --git a/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorTest.java b/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorTest.java index 2a74f59..639595d 100644 --- a/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorTest.java +++ b/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorTest.java @@ -11,7 +11,7 @@ import org.opencds.cqf.cql.ls.server.manager.CqlTranslationManager; import org.opencds.cqf.cql.ls.server.manager.TranslatorOptionsManager; import org.opencds.cqf.cql.ls.server.service.TestContentService; -import org.opencds.cqf.cql.ls.server.utility.OverlappingElements; +import org.opencds.cqf.cql.ls.server.utility.CursorOverlappingElements; import java.util.List; @@ -32,17 +32,12 @@ public static void beforeAll() { @Test public void shouldGetExpressionRefAtPos() throws Exception { - - ExpressionOverlapVisitor expressionOverlapVisitor = new ExpressionOverlapVisitor(); Position position = new Position(14, 28); - ExpressionOverlapVisitorContext context = new ExpressionOverlapVisitorContext(position); - expressionOverlapVisitor.visitLibrary(library, context); - - List overlappingElements = context.getOverlappingElements(); + List overlappingElements =CursorOverlappingElements.getElementsAtPosition(position, library); assertEquals(3, overlappingElements.size()); - Element specificElement = OverlappingElements.getMostSpecificElement(overlappingElements); + Element specificElement = CursorOverlappingElements.getMostSpecificElement(overlappingElements); assertTrue(specificElement instanceof ExpressionRef); ExpressionRef el = (ExpressionRef) specificElement; From 9bf6b2f63861c6b3b0656eec8d294660a8945ef2 Mon Sep 17 00:00:00 2001 From: Craig Sennabaum Date: Wed, 23 Nov 2022 07:18:14 -0500 Subject: [PATCH 4/5] remove comment --- .../provider/GoToDefinitionProvider.java | 34 +------------------ 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java index 03d47fe..0b0c033 100644 --- a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java +++ b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/provider/GoToDefinitionProvider.java @@ -63,39 +63,7 @@ public Either, List> getDefinit } protected LocationLink getLocationLinkOfElement(Element element, Library currentLibrary, URI currentUri) { - - // This case must come before ExpressionRef case because FunctionRef inherits from ExpressionRef -// if (element instanceof FunctionRef) { -// FunctionRef elFunctionRef = (FunctionRef) element; -// -// ImmutablePair searchLibraryPair; -// -// // If libraryName exists search for it, else use current. -// if (elFunctionRef.getLibraryName() == null) { -// searchLibraryPair = new ImmutablePair(currentUri, currentLibrary); -// } else { -// searchLibraryPair = findIncludedLibrary(currentLibrary, elFunctionRef.getLibraryName(), Uris.getHead(currentUri)); -// } -// -// if (searchLibraryPair != null) { -// -// List expressionDefCandidates = GoToDefinitionProvider.getExpressionDefCandidatesByName(searchLibraryPair.getRight(), elFunctionRef.getName()); -// -// // If exactly one match by name, don't worry about arguments matching -// if (expressionDefCandidates.size() == 1) { -// return GoToDefinitionProvider.getLocationLinkForExpressionDef(expressionDefCandidates.get(0), searchLibraryPair.getLeft(), elFunctionRef); -// } -// -// // If more than one match, try to match calling function args with definition func args -// else if (expressionDefCandidates.size() > 1) { -// FunctionDef functionDefCandidate = GoToDefinitionProvider.findMatchingFunctionDefInLibrary(expressionDefCandidates, elFunctionRef); -// if (functionDefCandidate != null) { -// return GoToDefinitionProvider.getLocationLinkForExpressionDef(functionDefCandidate, searchLibraryPair.getLeft(), elFunctionRef); -// } -// } -// } -// } - + if (element instanceof ExpressionRef) { ExpressionRef elExpressionRef = (ExpressionRef) element; From e97aa032c188728f4bbf25c2832184be0fe6f443 Mon Sep 17 00:00:00 2001 From: Craig Sennabaum Date: Wed, 23 Nov 2022 13:14:21 -0500 Subject: [PATCH 5/5] test for element sorting --- .../utility/CursorOverlappingElements.java | 14 ++--- .../CursorOverlappingElementsTest.java | 59 +++++++++++++++++++ .../visitor/ExpressionOverlapVisitorTest.java | 5 +- 3 files changed, 69 insertions(+), 9 deletions(-) create mode 100644 ls/server/src/test/java/org/opencds/cqf/cql/ls/server/utility/CursorOverlappingElementsTest.java diff --git a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/CursorOverlappingElements.java b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/CursorOverlappingElements.java index d1b97b3..f4b6aef 100644 --- a/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/CursorOverlappingElements.java +++ b/ls/server/src/main/java/org/opencds/cqf/cql/ls/server/utility/CursorOverlappingElements.java @@ -18,7 +18,11 @@ public class CursorOverlappingElements { public static Element getMostSpecificElementAtPosition (Position searchPosition, Library library) { List elements = CursorOverlappingElements.getElementsAtPosition(searchPosition, library); - return CursorOverlappingElements.getMostSpecificElement(elements); + if (elements.size() == 0) { + return null; + } + elements = CursorOverlappingElements.sortElementsFromSmallestToLargest(elements); + return elements.get(0); } public static List getElementsAtPosition (Position searchPosition, Library library) { @@ -28,11 +32,7 @@ public static List getElementsAtPosition (Position searchPosition, Libr return context.getOverlappingElements(); } - public static Element getMostSpecificElement(List elements) { - if (elements.size() == 0) { - return null; - } - + public static List sortElementsFromSmallestToLargest (List elements) { Comparator compareElements = (o1, o2) -> { if (o1.getTrackbacks().size() == 0) { return 1; @@ -74,6 +74,6 @@ public static Element getMostSpecificElement(List elements) { Collections.sort(elements, compareElements); - return elements.get(0); + return elements; } } diff --git a/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/utility/CursorOverlappingElementsTest.java b/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/utility/CursorOverlappingElementsTest.java new file mode 100644 index 0000000..d5f2a6c --- /dev/null +++ b/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/utility/CursorOverlappingElementsTest.java @@ -0,0 +1,59 @@ +package org.opencds.cqf.cql.ls.server.utility; + +import org.cqframework.cql.elm.tracking.TrackBack; +import org.eclipse.lsp4j.Position; +import org.hl7.elm.r1.Element; +import org.hl7.elm.r1.ExpressionRef; +import org.hl7.elm.r1.Library; +import org.hl7.elm.r1.Literal; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class CursorOverlappingElementsTest { + @Test + public void shouldSortElementsFromSmallestToLargest() throws Exception { + + List overlappingElements = new ArrayList<>(); + + Library lib = new Library(); + + Literal one = new Literal(); + one.getTrackbacks().add(new TrackBack(lib.getIdentifier(), 1, 1, 5, 10)); + + Literal two = new Literal(); + two.getTrackbacks().add(new TrackBack(lib.getIdentifier(), 1, 1, 4, 10)); + + Literal three = new Literal(); + three.getTrackbacks().add(new TrackBack(lib.getIdentifier(), 1, 5, 4, 10)); + + Literal four = new Literal(); + four.getTrackbacks().add(new TrackBack(lib.getIdentifier(), 2, 5, 4, 10)); + + Literal five = new Literal(); + five.getTrackbacks().add(new TrackBack(lib.getIdentifier(), 3, 5, 3, 10)); + + Literal six = new Literal(); + six.getTrackbacks().add(new TrackBack(lib.getIdentifier(), 3, 5, 3, 8)); + + // Place them in wrong order + overlappingElements.add(two); + overlappingElements.add(five); + overlappingElements.add(six); + overlappingElements.add(one); + overlappingElements.add(three); + overlappingElements.add(four); + + List sortedElements = CursorOverlappingElements.sortElementsFromSmallestToLargest(overlappingElements); + assertEquals(sortedElements.get(0), one); + assertEquals(sortedElements.get(1), two); + assertEquals(sortedElements.get(2), three); + assertEquals(sortedElements.get(3), four); + assertEquals(sortedElements.get(4), five); + assertEquals(sortedElements.get(5), six); + } +} diff --git a/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorTest.java b/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorTest.java index 639595d..c92400f 100644 --- a/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorTest.java +++ b/ls/server/src/test/java/org/opencds/cqf/cql/ls/server/visitor/ExpressionOverlapVisitorTest.java @@ -12,6 +12,7 @@ import org.opencds.cqf.cql.ls.server.manager.TranslatorOptionsManager; import org.opencds.cqf.cql.ls.server.service.TestContentService; import org.opencds.cqf.cql.ls.server.utility.CursorOverlappingElements; +import org.opencds.cqf.cql.ls.server.utility.CursorOverlappingElementsTest; import java.util.List; @@ -34,10 +35,10 @@ public static void beforeAll() { public void shouldGetExpressionRefAtPos() throws Exception { Position position = new Position(14, 28); - List overlappingElements =CursorOverlappingElements.getElementsAtPosition(position, library); + List overlappingElements = CursorOverlappingElements.getElementsAtPosition(position, library); assertEquals(3, overlappingElements.size()); - Element specificElement = CursorOverlappingElements.getMostSpecificElement(overlappingElements); + Element specificElement = CursorOverlappingElements.getMostSpecificElementAtPosition(position, library); assertTrue(specificElement instanceof ExpressionRef); ExpressionRef el = (ExpressionRef) specificElement;