Fix rascal hovers showing random types

rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/model/FileFacts.java

@@ -112,7 +112,7 @@ public FileFact(ISourceLocation file, Executor exec) {
                 r -> {
                     r.interrupt();
                     InterruptibleFuture<@Nullable SummaryBridge> summaryCalc = rascal.getSummary(file, confs.lookupConfig(file))
-                        .thenApply(s -> s == null ? null : new SummaryBridge(s, cm));
+                        .thenApply(s -> s == null ? null : new SummaryBridge(file, s, cm));
                     // only run get summary after the typechecker for this file is done running
                     // (we cannot now global running type checkers, that is a different subject)
                     CompletableFuture<@Nullable SummaryBridge> mergedCalc = typeCheckResults.get().thenCompose(o -> summaryCalc.get());

rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/model/SummaryBridge.java

@@ -78,18 +78,23 @@ public SummaryBridge() {
         this.typeNames = TreeMapLookup::new;
     }
 
-    public SummaryBridge(IConstructor summary, ColumnMaps cm) {
+    public SummaryBridge(ISourceLocation self, IConstructor summary, ColumnMaps cm) {
         this.data = summary.asWithKeywordParameters();
-        definitions = Lazy.defer(() -> translateRelation(getKWFieldSet(data, "useDef"), v -> Locations.toLSPLocation((ISourceLocation)v, cm), cm));
-        typeNames = Lazy.defer(() -> translateMap(getKWFieldMap(data, "locationTypes"), v -> ((IString)v).getValue(), cm));
+        definitions = Lazy.defer(() -> translateRelation(getKWFieldSet(data, "useDef"), self, v -> Locations.toLSPLocation((ISourceLocation)v, cm), cm));
+        typeNames = Lazy.defer(() -> translateMap(getKWFieldMap(data, "locationTypes"), self, v -> ((IString)v).getValue(), cm));
 
     }
 
-    private static <T> IRangeMap<List<T>> translateRelation(ISet binaryRel, Function<IValue, T> valueMapper, ColumnMaps cm) {
+    private static <T> IRangeMap<List<T>> translateRelation(ISet binaryRel, ISourceLocation self, Function<IValue, T> valueMapper, ColumnMaps cm) {
         TreeMapLookup<List<T>> result = new TreeMapLookup<>();
         for (IValue v: binaryRel) {
             ITuple row = (ITuple)v;
-            Range from = Locations.toRange((ISourceLocation)row.get(0), cm);
+            ISourceLocation fromLoc = (ISourceLocation)row.get(0);
+            if (!fromLoc.top().equals(self)) {
+                // ignore rascal-core giving us entries that are not starting with our own base
+                continue;
+            }
+            Range from = Locations.toRange(fromLoc, cm);
             T to = valueMapper.apply(row.get(1));
             List<T> existing = result.getExact(from);
             if (existing == null) {
@@ -109,12 +114,16 @@ else if (existing.size() == 1) {
         return result;
     }
 
-    private static <T> IRangeMap<T> translateMap(IMap binaryMap, Function<IValue, T> valueMapper, ColumnMaps cm) {
+    private static <T> IRangeMap<T> translateMap(IMap binaryMap, ISourceLocation self, Function<IValue, T> valueMapper, ColumnMaps cm) {
         TreeMapLookup<T> result = new TreeMapLookup<>();
         binaryMap.entryIterator().forEachRemaining(e -> {
-            Range from = Locations.toRange((ISourceLocation)e.getKey(), cm);
-            T to = valueMapper.apply(e.getValue());
-            result.put(from, to);
+            var fromLoc = (ISourceLocation)e.getKey();
+            if (fromLoc.top().equals(self)) {
+                // only build a map for our own entries (rascal-core reports too much)
+                Range from = Locations.toRange(fromLoc, cm);
+                T to = valueMapper.apply(e.getValue());
+                result.put(from, to);
+            }
         });
         return result;
     }

rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/locations/impl/TreeMapLookup.java

@@ -63,7 +63,7 @@ private static int compareRanges(Range a, Range b) {
             }
             return Integer.compare(aEnd.getLine(), bEnd.getLine());
         }
-        return Integer.compare(aStart.getLine(), aStart.getLine());
+        return Integer.compare(aStart.getLine(), bStart.getLine());
     }
 
     private static boolean rangeContains(Range a, Range b) {
@@ -101,12 +101,18 @@ private static boolean rangeContains(Range a, Range b) {
 
     @Override
     public @Nullable T lookup(Range from) {
-        T result = contains(data.floorEntry(from), from);
-        if (result == null) {
-            // could be that it's at the start of the entry
-            result = contains(data.ceilingEntry(from), from);
+        // since we allow for overlapping ranges, it might be that we have to
+        // search all the way to the "bottom" of the tree to see if we are
+        // contained in something larger than the closest key
+        var previousKeys = data.headMap(from, true).descendingMap();
+        for (var candidate : previousKeys.entrySet()) {
+            T result = contains(candidate, from);
+            if (result != null) {
+                return result;
+            }
         }
-        return result;
+        // could be that it's at the start of the entry (so the entry it not in the head map)
+        return contains(data.ceilingEntry(from), from);
     }
 
     @Override

rascal-lsp/src/test/java/engineering/swat/rascal/lsp/util/LookupTests.java

@@ -27,7 +27,9 @@
 package engineering.swat.rascal.lsp.util;
 
 import static org.junit.jupiter.api.Assertions.assertSame;
-
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Random;
 import org.eclipse.lsp4j.Position;
 import org.eclipse.lsp4j.Range;
 import org.junit.jupiter.api.Test;
@@ -121,4 +123,42 @@ public void testSimpleLookupEnd2() {
         assertSame("hit2", target.lookup(cursor(1,12)));
     }
 
+    @Test
+    public void testOverlappingRanges() {
+        TreeMapLookup<String> target = new TreeMapLookup<>();
+        target.put(new Range(new Position(0,1), new Position(4,5)), "big1");
+        target.put(new Range(new Position(1,1), new Position(1,5)), "small1");
+        target.put(new Range(new Position(4,1), new Position(4,4)), "small2");
+        assertSame("big1", target.lookup(cursor(0,12)));
+        assertSame("big1", target.lookup(cursor(1,12)));
+        assertSame("small1", target.lookup(cursor(1,4)));
+        assertSame("small2", target.lookup(cursor(4,4)));
+        assertSame("big1", target.lookup(cursor(4,5)));
+    }
+
+    @Test
+    public void randomRanges() {
+        TreeMapLookup<String> target = new TreeMapLookup<>();
+        Map<Range, String> ranges = new HashMap<>();
+        Random r = new Random();
+        for (int i = 0; i < 1000; i++) {
+            int startLine = r.nextInt(10);
+            int startColumn = r.nextInt(10);
+            int endLine = startLine + r.nextInt(3);
+            int endColumn;
+            if (endLine == startLine) {
+                endColumn = startColumn + r.nextInt(3);
+            }
+            else {
+                endColumn = r.nextInt(10);
+            }
+            ranges.put(range(startLine, startColumn, endLine, endColumn), "" + i);
+        }
+        ranges.forEach(target::put);
+        for (var e: ranges.entrySet()) {
+            var found = target.lookup(e.getKey());
+            assertSame(e.getValue(), found, "Entry " + e + "should be found");
+        }
+    }
+
 }