Skip to content

Commit

Permalink
Deliver full intersection type of a value to the IDE (#11583)
Browse files Browse the repository at this point in the history
close #11481

Changelog:
- update: handle `MultiTypeValue` results in the execution instrument
- update: language server protocol supports multi-type values
- update: GUI uses only the first type of a multi-type value when

# Important Notes
GUI uses only the first type of the intersection. See the difference between `Integer&Text` and `Text&Integer`:

https://github.com/user-attachments/assets/29efc89b-c223-4043-8dff-9cdae1987f0c
  • Loading branch information
4e6 authored Nov 22, 2024
1 parent 789eab8 commit 2b9ed57
Show file tree
Hide file tree
Showing 24 changed files with 327 additions and 524 deletions.
2 changes: 1 addition & 1 deletion app/gui/src/project-view/stores/graph/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ export const { injectFn: useGraphStore, provideFn: provideGraphStore } = createC
profilingInfo: update.profilingInfo ?? [],
fromCache: update.fromCache ?? false,
payload: update.payload ?? { type: 'Value' },
...(update.type ? { type: update.type } : {}),
type: update.type ?? [],
...(update.methodCall ? { methodCall: update.methodCall } : {}),
}
proj.computedValueRegistry.processUpdates([update_])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ function updateInfo(info: ExpressionInfo, update: ExpressionUpdate) {

function combineInfo(info: ExpressionInfo | undefined, update: ExpressionUpdate): ExpressionInfo {
const isPending = update.payload.type === 'Pending'
const updateSingleValueType = update.type.at(0) // TODO: support multi-value (aka intersection) types
return {
typename: update.type ?? (isPending ? info?.typename : undefined),
typename: updateSingleValueType ?? (isPending ? info?.typename : undefined),
methodCall: update.methodCall ?? (isPending ? info?.methodCall : undefined),
payload: update.payload,
profilingInfo: update.profilingInfo,
Expand Down
2 changes: 1 addition & 1 deletion app/ydoc-shared/src/languageServerTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ export interface ExpressionUpdate {
/** The id of updated expression. */
expressionId: ExpressionId
/** The updated type of the expression. */
type?: string
type: string[]
/** The updated method call info. */
methodCall?: MethodCall
/** Profiling information about the expression. */
Expand Down
10 changes: 8 additions & 2 deletions docs/language-server/protocol-language-server.md
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,14 @@ An update about the computed expression.
interface ExpressionUpdate {
/** The id of updated expression. */
expressionId: ExpressionId;
/** The updated type of the expression. */
type?: string;
/** The updated type of the expression.
*
* Possible values:
* - empty array indicates no type information for this expression
* - array with a single value contains a value of this expression
* - array with multiple values represents an intersetion type
*/
type: string[];
/** The updated method call info. */
methodCall?: MethodCall;
/** Profiling information about the expression. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ final class ContextEventsListener(
val computedExpressions = expressionUpdates.map { update =>
ContextRegistryProtocol.ExpressionUpdate(
update.expressionId,
update.expressionType,
update.expressionTypes.getOrElse(Vector()),
update.methodCall.map(toProtocolMethodCall),
update.profilingInfo.map(toProtocolProfilingInfo),
update.fromCache,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,15 @@ object ContextRegistryProtocol {
/** An update about computed expression.
*
* @param expressionId the id of updated expression
* @param `type` the updated type of expression
* @param type the updated type of expression
* @param methodCall the updated method call
* @param profilingInfo profiling information about the expression
* @param fromCache whether or not the expression's value came from the cache
* @param fromCache whether the expression's value came from the cache
* @param payload an extra information about the computed value
*/
case class ExpressionUpdate(
expressionId: UUID,
`type`: Option[String],
`type`: Vector[String],
methodCall: Option[MethodCall],
profilingInfo: Vector[ProfilingInfo],
fromCache: Boolean,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ final class SuggestionsHandler(
config,
suggestionsRepo
)
context.system.eventStream
.subscribe(self, classOf[Api.ExpressionUpdates])
context.system.eventStream
.subscribe(self, classOf[Api.SuggestionsDatabaseModuleUpdateNotification])
context.system.eventStream.subscribe(
Expand Down Expand Up @@ -264,54 +262,6 @@ final class SuggestionsHandler(
)
)

case msg: Api.ExpressionUpdates if state.isSuggestionUpdatesRunning =>
state.suggestionUpdatesQueue.enqueue(msg)

case Api.ExpressionUpdates(_, updates) =>
logger.debug(
"Received expression updates [{}]",
updates.map(u => (u.expressionId, u.expressionType))
)
val types = updates.toSeq
.filter(_.typeChanged)
.flatMap(update => update.expressionType.map(update.expressionId -> _))
suggestionsRepo
.updateAll(types)
.map { case (version, updatedIds) =>
val updates = types.zip(updatedIds).collect {
case ((_, typeValue), Some(suggestionId)) =>
SuggestionsDatabaseUpdate.Modify(
id = suggestionId,
returnType = Some(fieldUpdate(typeValue))
)
}
SuggestionsDatabaseUpdateNotification(version, updates)
}
.onComplete {
case Success(notification) =>
if (notification.updates.nonEmpty) {
clients.foreach { clientId =>
sessionRouter ! DeliverToJsonController(clientId, notification)
}
}
self ! SuggestionsHandler.SuggestionUpdatesCompleted
case Failure(ex) =>
logger.error(
"Error applying changes from computed values [{}]",
updates.map(_.expressionId),
ex
)
self ! SuggestionsHandler.SuggestionUpdatesCompleted
}
context.become(
initialized(
projectName,
graph,
clients,
state.suggestionUpdatesRunning()
)
)

case Api.BackgroundJobsStartedNotification() =>
self ! SuggestionLoadingCompleted
context.become(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class ContextEventsListenerSpec
Set(
Api.ExpressionUpdate(
Suggestions.method.externalId.get,
Some(Suggestions.method.returnType),
Some(Vector(Suggestions.method.returnType)),
Some(methodCall),
Vector(),
false,
Expand All @@ -86,7 +86,7 @@ class ContextEventsListenerSpec
Vector(
ContextRegistryProtocol.ExpressionUpdate(
Suggestions.method.externalId.get,
Some(Suggestions.method.returnType),
Vector(Suggestions.method.returnType),
Some(toProtocolMethodCall(methodCall)),
Vector(),
false,
Expand Down Expand Up @@ -135,7 +135,7 @@ class ContextEventsListenerSpec
Vector(
ContextRegistryProtocol.ExpressionUpdate(
Suggestions.method.externalId.get,
None,
Vector(),
None,
Vector(),
false,
Expand Down Expand Up @@ -173,7 +173,7 @@ class ContextEventsListenerSpec
Vector(
ContextRegistryProtocol.ExpressionUpdate(
Suggestions.method.externalId.get,
None,
Vector(),
None,
Vector(),
false,
Expand Down Expand Up @@ -229,7 +229,7 @@ class ContextEventsListenerSpec
Vector(
ContextRegistryProtocol.ExpressionUpdate(
Suggestions.method.externalId.get,
None,
Vector(),
None,
Vector(),
false,
Expand All @@ -238,7 +238,7 @@ class ContextEventsListenerSpec
),
ContextRegistryProtocol.ExpressionUpdate(
Suggestions.local.externalId.get,
None,
Vector(),
None,
Vector(),
false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,20 +108,18 @@ object Runtime {
/** An update about the computed expression.
*
* @param expressionId the expression id
* @param expressionType the type of expression
* @param expressionTypes the type of expression
* @param methodCall the underlying method call of this expression
* @param profilingInfo profiling information about the execution of this
* expression
* @param fromCache whether or not the value for this expression came
* from the cache
* @param typeChanged whether or not the type of the value or method definition
* @param profilingInfo profiling information about the execution of this expression
* @param fromCache whether the value for this expression came from the cache
* @param typeChanged whether the type of the value or method definition
* has changed from the one that was cached, if any
* @param payload an extra information about the computed value
*/
@named("expressionUpdate")
case class ExpressionUpdate(
expressionId: ExpressionId,
expressionType: Option[String],
expressionTypes: Option[Vector[String]],
methodCall: Option[MethodCall],
profilingInfo: Vector[ProfilingInfo],
fromCache: Boolean,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
public final class RuntimeCache implements java.util.function.Function<String, Object> {
private final Map<UUID, Reference<Object>> cache = new HashMap<>();
private final Map<UUID, Reference<Object>> expressions = new HashMap<>();
private final Map<UUID, String> types = new HashMap<>();
private final Map<UUID, String[]> types = new HashMap<>();
private final Map<UUID, ExecutionService.FunctionCallInfo> calls = new HashMap<>();
private CachePreferences preferences = CachePreferences.empty();
private Consumer<UUID> observer;
Expand Down Expand Up @@ -107,15 +107,15 @@ public Set<UUID> clear(CachePreferences.Kind kind) {
* @return the previously cached type.
*/
@CompilerDirectives.TruffleBoundary
public String putType(UUID key, String typeName) {
return types.put(key, typeName);
public String[] putType(UUID key, String[] typeNames) {
return types.put(key, typeNames);
}

/**
* @return the cached type of the expression
*/
@CompilerDirectives.TruffleBoundary
public String getType(UUID key) {
public String[] getType(UUID key) {
return types.get(key);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,16 @@ public Object findCachedResult(IdExecutionService.Info info) {
@Override
public void updateCachedResult(IdExecutionService.Info info) {
Object result = info.getResult();
String resultType = typeOf(result);
String[] resultTypes = typeOf(result);
UUID nodeId = info.getId();
String cachedType = cache.getType(nodeId);
String[] cachedTypes = cache.getType(nodeId);
FunctionCallInfo call = functionCallInfoById(nodeId);
FunctionCallInfo cachedCall = cache.getCall(nodeId);
ProfilingInfo[] profilingInfo = new ProfilingInfo[] {new ExecutionTime(info.getElapsedTime())};

ExpressionValue expressionValue =
new ExpressionValue(
nodeId, result, resultType, cachedType, call, cachedCall, profilingInfo, false);
nodeId, result, resultTypes, cachedTypes, call, cachedCall, profilingInfo, false);
syncState.setExpressionUnsync(nodeId);
syncState.setVisualizationUnsync(nodeId);

Expand All @@ -119,7 +119,7 @@ public void updateCachedResult(IdExecutionService.Info info) {
cache.offer(nodeId, result);
cache.putCall(nodeId, call);
}
cache.putType(nodeId, resultType);
cache.putType(nodeId, resultTypes);

callOnComputedCallback(expressionValue);
executeOneshotExpressions(nodeId, result, info);
Expand Down Expand Up @@ -214,20 +214,22 @@ private FunctionCallInfo functionCallInfoById(UUID nodeId) {
return calls.get(nodeId);
}

private String typeOf(Object value) {
String resultType;
private String[] typeOf(Object value) {
if (value instanceof UnresolvedSymbol) {
resultType = Constants.UNRESOLVED_SYMBOL;
} else {
var typeOfNode = TypeOfNode.getUncached();
Object typeResult = value == null ? null : typeOfNode.findTypeOrError(value);
if (typeResult instanceof Type t) {
resultType = getTypeQualifiedName(t);
} else {
resultType = null;
return new String[] {Constants.UNRESOLVED_SYMBOL};
}

var typeOfNode = TypeOfNode.getUncached();
Type[] allTypes = value == null ? null : typeOfNode.findAllTypesOrNull(value);
if (allTypes != null) {
String[] result = new String[allTypes.length];
for (var i = 0; i < allTypes.length; i++) {
result[i] = getTypeQualifiedName(allTypes[i]);
}
return result;
}
return resultType;

return null;
}

@CompilerDirectives.TruffleBoundary
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -661,8 +661,8 @@ public FunctionCallInstrumentationNode.FunctionCall getCall() {
public static final class ExpressionValue {
private final UUID expressionId;
private final Object value;
private final String type;
private final String cachedType;
private final String[] types;
private final String[] cachedTypes;
private final FunctionCallInfo callInfo;
private final FunctionCallInfo cachedCallInfo;
private final ProfilingInfo[] profilingInfo;
Expand All @@ -673,8 +673,8 @@ public static final class ExpressionValue {
*
* @param expressionId the id of the expression being computed.
* @param value the value returned by computing the expression.
* @param type the type of the returned value.
* @param cachedType the cached type of the value.
* @param types the type of the returned value.
* @param cachedTypes the cached type of the value.
* @param callInfo the function call data.
* @param cachedCallInfo the cached call data.
* @param profilingInfo the profiling information associated with this node
Expand All @@ -683,16 +683,16 @@ public static final class ExpressionValue {
public ExpressionValue(
UUID expressionId,
Object value,
String type,
String cachedType,
String[] types,
String[] cachedTypes,
FunctionCallInfo callInfo,
FunctionCallInfo cachedCallInfo,
ProfilingInfo[] profilingInfo,
boolean wasCached) {
this.expressionId = expressionId;
this.value = value;
this.type = type;
this.cachedType = cachedType;
this.types = types;
this.cachedTypes = cachedTypes;
this.callInfo = callInfo;
this.cachedCallInfo = cachedCallInfo;
this.profilingInfo = profilingInfo;
Expand All @@ -707,11 +707,11 @@ public String toString() {
+ expressionId
+ ", value="
+ (value == null ? "null" : new MaskedString(value.toString()).applyMasking())
+ ", type='"
+ type
+ ", types='"
+ Arrays.toString(types)
+ '\''
+ ", cachedType='"
+ cachedType
+ ", cachedTypes='"
+ Arrays.toString(cachedTypes)
+ '\''
+ ", callInfo="
+ callInfo
Expand All @@ -734,15 +734,15 @@ public UUID getExpressionId() {
/**
* @return the type of the returned value.
*/
public String getType() {
return type;
public String[] getTypes() {
return types;
}

/**
* @return the cached type of the value.
*/
public String getCachedType() {
return cachedType;
public String[] getCachedTypes() {
return cachedTypes;
}

/**
Expand Down Expand Up @@ -784,7 +784,7 @@ public boolean wasCached() {
* @return {@code true} when the type differs from the cached value.
*/
public boolean isTypeChanged() {
return !Objects.equals(type, cachedType);
return !Arrays.equals(types, cachedTypes);
}

/**
Expand Down
Loading

0 comments on commit 2b9ed57

Please sign in to comment.