Skip to content

Commit

Permalink
GH-2228: (Non-static) polyfills throwing exception in case of several…
Browse files Browse the repository at this point in the history
… polyfills for same type per project (#2229)

* tentative fix for the bug

* add a test

* alternative approach (experimental)
  • Loading branch information
mor-n4 authored Oct 14, 2021
1 parent 1060ad0 commit 3dc450a
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ class N4JSClassValidator extends AbstractN4JSDeclarativeValidator implements Pol
return;
}

if (polyfillValidatorFragment.holdsPolyfill(this, n4Class)) {
if (polyfillValidatorFragment.holdsPolyfill(this, n4Class, getCancelIndicator())) {
val tClass = n4Class.definedType as TClass;
internalCheckAbstractFinal(tClass);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class N4JSInterfaceValidator extends AbstractN4JSDeclarativeValidator implements
return;
}

if (polyfillValidatorFragment.holdsPolyfill(this, n4Interface)) {
if (polyfillValidatorFragment.holdsPolyfill(this, n4Interface, getCancelIndicator())) {

holdsNoCyclicInheritance(n4Interface)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,15 @@

import org.eclipse.emf.common.util.EList;
import org.eclipse.emf.ecore.EObject;
import org.eclipse.emf.ecore.resource.Resource;
import org.eclipse.emf.ecore.util.EcoreUtil;
import org.eclipse.n4js.N4JSGlobals;
import org.eclipse.n4js.n4JS.N4ClassDeclaration;
import org.eclipse.n4js.n4JS.N4ClassifierDeclaration;
import org.eclipse.n4js.n4JS.N4InterfaceDeclaration;
import org.eclipse.n4js.n4JS.N4JSPackage;
import org.eclipse.n4js.n4JS.TypeReferenceNode;
import org.eclipse.n4js.resource.N4JSResource;
import org.eclipse.n4js.scoping.utils.PolyfillUtils;
import org.eclipse.n4js.ts.typeRefs.ParameterizedTypeRef;
import org.eclipse.n4js.ts.typeRefs.TypeArgument;
Expand All @@ -81,6 +83,7 @@
import org.eclipse.xtext.resource.IResourceDescriptions;
import org.eclipse.xtext.resource.XtextResource;
import org.eclipse.xtext.resource.impl.ResourceDescriptionsProvider;
import org.eclipse.xtext.util.CancelIndicator;
import org.eclipse.xtext.xbase.lib.IterableExtensions;

import com.google.common.base.Joiner;
Expand Down Expand Up @@ -114,6 +117,7 @@ public class PolyfillValidatorFragment {
private static class PolyfillValidationState {
/** A {@link N4JSClassValidator} or {@link N4JSInterfaceValidator}, used for adding issues. */
PolyfillValidatorHost host;
CancelIndicator cancelIndicator;
N4ClassifierDeclaration n4Classifier;
TypeReferenceNode<ParameterizedTypeRef> superClassifierNode;
TN4Classifier polyType;
Expand All @@ -129,11 +133,13 @@ private void addIssue(PolyfillValidationState state, String msg, String issueCod
* Checks polyfill constraints on given class declaration using validator to issue errors. Constraints (Polyfill
* Class) 156: Polyfill
*/
public boolean holdsPolyfill(PolyfillValidatorHost validator, N4ClassifierDeclaration n4Classifier) {
public boolean holdsPolyfill(PolyfillValidatorHost validator, N4ClassifierDeclaration n4Classifier,
CancelIndicator cancelIndicator) {
boolean isStaticPolyFill = N4JSLanguageUtils.isStaticPolyfill(n4Classifier);
if (isStaticPolyFill || N4JSLanguageUtils.isNonStaticPolyfill(n4Classifier)) {
PolyfillValidationState state = new PolyfillValidationState();
state.host = validator;
state.cancelIndicator = cancelIndicator;
state.n4Classifier = n4Classifier;
state.name = n4Classifier.getName();

Expand Down Expand Up @@ -447,6 +453,12 @@ private boolean holdsSinglePolyfillSource(PolyfillValidationState state) {
// Resolve
if (eob.eIsProxy()) {
eob = EcoreUtil.resolve(eob, res);
if (!eob.eIsProxy()) {
Resource filledResource = eob.eResource();
if (filledResource instanceof N4JSResource) {
((N4JSResource) filledResource).performPostProcessing(state.cancelIndicator);
}
}
}

if (eob == state.polyType) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"name": "Test",
"version": "0.0.1",
"n4js": {
"projectType": "runtimeLibrary",
"vendorId": "org.eclipse.n4js",
"vendorName": "Eclipse N4JS Project",
"sources": {
"source": [
"src"
]
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright (c) 2019 NumberFour AG.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* NumberFour AG - Initial API and implementation
*/

/* XPECT_SETUP org.eclipse.n4js.xpect.tests.N4jsXtTest
Workspace {
Project "Test" {
Folder "src" {
File "Other.n4jsd" { from="./Other.n4jsd" }
ThisFile {}
}
File "package.json" { from="../package.json" }
}
}
END_SETUP
*/

@@Global @@ProvidedByRuntime

// XPECT noerrors -->
@Polyfill export external public class Array<T> extends Array<T> {
// no members required here
}

// before the bug fix the following exception was thrown:
//
// Caused by: java.lang.IllegalArgumentException: name may not be null
// at org.eclipse.n4js.ts.types.util.NameStaticPair.<init>(NameStaticPair.java:48)
// at org.eclipse.n4js.ts.types.util.NameStaticPair.<init>(NameStaticPair.java:40)
// at org.eclipse.n4js.ts.types.util.NameStaticPair.of(NameStaticPair.java:33)
// at org.eclipse.n4js.validation.utils.MemberCube.addMembers(MemberCube.java:52)
// at org.eclipse.n4js.validation.utils.MemberCube.<init>(MemberCube.java:43)
// at org.eclipse.n4js.validation.validators.N4JSMemberRedefinitionValidator.createMemberValidationList(N4JSMemberRedefinitionValidator.java:1278)
// at org.eclipse.n4js.validation.validators.N4JSMemberRedefinitionValidator.checkMemberRedefinitions(N4JSMemberRedefinitionValidator.java:189)
// at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
// at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
// at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
// at java.base/java.lang.reflect.Method.invoke(Method.java:566)
// at org.eclipse.xtext.validation.AbstractDeclarativeValidator$MethodWrapper.invoke(AbstractDeclarativeValidator.java:129)
// at org.eclipse.n4js.validation.N4JSValidator$N4JSMethodWrapperCancelable.invoke(N4JSValidator.java:105)
// ... 32 more
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright (c) 2021 NumberFour AG.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* NumberFour AG - Initial API and implementation
*/

@@Global @@ProvidedByRuntime

@Polyfill export external public class Array<T> extends Array<T> {
public [Symbol.hasInstance](): any+;
}

0 comments on commit 3dc450a

Please sign in to comment.