From 687866de8e56d2d2a508100694f9b2cfbedc19c9 Mon Sep 17 00:00:00 2001 From: Lorenzo Bettini Date: Thu, 18 Jan 2024 17:44:40 +0100 Subject: [PATCH 1/4] test for reproducing the wrong position of marker it also covers several supertypes, which was not covered before --- .../validation/OverrideValidationTest.java | 11 +++++++---- .../test/ExceptionThrowingInterface2.java | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 org.eclipse.xtend.core.tests/testdata/test/ExceptionThrowingInterface2.java diff --git a/org.eclipse.xtend.core.tests/src/org/eclipse/xtend/core/tests/validation/OverrideValidationTest.java b/org.eclipse.xtend.core.tests/src/org/eclipse/xtend/core/tests/validation/OverrideValidationTest.java index 091dee92461..516ce759662 100644 --- a/org.eclipse.xtend.core.tests/src/org/eclipse/xtend/core/tests/validation/OverrideValidationTest.java +++ b/org.eclipse.xtend.core.tests/src/org/eclipse/xtend/core/tests/validation/OverrideValidationTest.java @@ -514,15 +514,18 @@ public void testClassMustBeAbstract_06() throws Exception { } /** - * Two incompatible exceptions; the marker is on the first one + * Three incompatible exceptions from different supertypes; the marker is on the first one */ @Test public void testIncompatibleThrowsClause_06() throws Exception { - var source = "class Foo extends test.ExceptionThrowing { override nullPointerException() throws java.io.IOException, java.io.FileNotFoundException {} }"; + var source = "class Foo extends test.ExceptionThrowing implements test.ExceptionThrowingInterface, test.ExceptionThrowingInterface2 {" + + "override nullPointerException() throws NullPointerException, java.io.IOException, java.io.FileNotFoundException {} " + + "}"; XtendClass xtendClass = clazz(source); helper.assertError(xtendClass.getMembers().get(0), XTEND_FUNCTION, INCOMPATIBLE_THROWS_CLAUSE, - source.indexOf("java.io.IOException"), "java.io.IOException".length(), + source.indexOf("NullPointerException"), "NullPointerException".length(), "declared exceptions", "IOException", "FileNotFoundException", - "are not", "compatible", "throws", "clause"); + "are not compatible with throws clause in " + + "ExceptionThrowing.nullPointerException(), ExceptionThrowingInterface.nullPointerException() and ExceptionThrowingInterface2.nullPointerException()"); } @Test public void testCompatibleThrowsClause() throws Exception { diff --git a/org.eclipse.xtend.core.tests/testdata/test/ExceptionThrowingInterface2.java b/org.eclipse.xtend.core.tests/testdata/test/ExceptionThrowingInterface2.java new file mode 100644 index 00000000000..09ea1ec0014 --- /dev/null +++ b/org.eclipse.xtend.core.tests/testdata/test/ExceptionThrowingInterface2.java @@ -0,0 +1,18 @@ +/******************************************************************************* + * Copyright (c) 2024 itemis AG (http://www.itemis.eu) and others. + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package test; + +/** + * @author Lorenzo Bettini - Initial contribution and API + */ + interface ExceptionThrowingInterface2 { + + void nullPointerException() throws NullPointerException ; + + } From a9233d3d67310b7fb48a35685573628b48f0313f Mon Sep 17 00:00:00 2001 From: Lorenzo Bettini Date: Thu, 18 Jan 2024 18:23:48 +0100 Subject: [PATCH 2/4] place error marker on each incompatible exceptions https://github.com/eclipse/xtext/issues/2912 --- .../validation/OverrideValidationTest.java | 16 +++-- .../xtend/core/validation/XtendValidator.java | 59 ++++++++----------- 2 files changed, 35 insertions(+), 40 deletions(-) diff --git a/org.eclipse.xtend.core.tests/src/org/eclipse/xtend/core/tests/validation/OverrideValidationTest.java b/org.eclipse.xtend.core.tests/src/org/eclipse/xtend/core/tests/validation/OverrideValidationTest.java index 516ce759662..6ad7298ed7e 100644 --- a/org.eclipse.xtend.core.tests/src/org/eclipse/xtend/core/tests/validation/OverrideValidationTest.java +++ b/org.eclipse.xtend.core.tests/src/org/eclipse/xtend/core/tests/validation/OverrideValidationTest.java @@ -514,18 +514,24 @@ public void testClassMustBeAbstract_06() throws Exception { } /** - * Three incompatible exceptions from different supertypes; the marker is on the first one + * Three incompatible exceptions from different supertypes; + * the marker is set on the offending exception */ @Test public void testIncompatibleThrowsClause_06() throws Exception { var source = "class Foo extends test.ExceptionThrowing implements test.ExceptionThrowingInterface, test.ExceptionThrowingInterface2 {" + "override nullPointerException() throws NullPointerException, java.io.IOException, java.io.FileNotFoundException {} " + "}"; XtendClass xtendClass = clazz(source); + var expectedSuffix = " is not compatible with throws clause in " + + "ExceptionThrowing.nullPointerException(), ExceptionThrowingInterface.nullPointerException() and ExceptionThrowingInterface2.nullPointerException()"; helper.assertError(xtendClass.getMembers().get(0), XTEND_FUNCTION, INCOMPATIBLE_THROWS_CLAUSE, - source.indexOf("NullPointerException"), "NullPointerException".length(), - "declared exceptions", "IOException", "FileNotFoundException", - "are not compatible with throws clause in " - + "ExceptionThrowing.nullPointerException(), ExceptionThrowingInterface.nullPointerException() and ExceptionThrowingInterface2.nullPointerException()"); + source.indexOf("java.io.IOException"), "java.io.IOException".length(), + "declared exception IOException", + expectedSuffix); + helper.assertError(xtendClass.getMembers().get(0), XTEND_FUNCTION, INCOMPATIBLE_THROWS_CLAUSE, + source.indexOf("java.io.FileNotFoundException"), "java.io.FileNotFoundException".length(), + "declared exception FileNotFoundException", + expectedSuffix); } @Test public void testCompatibleThrowsClause() throws Exception { diff --git a/org.eclipse.xtend.core/src/org/eclipse/xtend/core/validation/XtendValidator.java b/org.eclipse.xtend.core/src/org/eclipse/xtend/core/validation/XtendValidator.java index 9e4172c3645..574bc2b9c06 100644 --- a/org.eclipse.xtend.core/src/org/eclipse/xtend/core/validation/XtendValidator.java +++ b/org.eclipse.xtend.core/src/org/eclipse/xtend/core/validation/XtendValidator.java @@ -872,14 +872,7 @@ else if(member instanceof XtendField) else return null; } - - protected EStructuralFeature exceptionsFeature(EObject member) { - if (member instanceof XtendExecutable) - return XTEND_EXECUTABLE__EXCEPTIONS; - else - return null; - } - + protected EStructuralFeature returnTypeFeature(EObject member) { if (member instanceof XtendFunction) return XTEND_FUNCTION__RETURN_TYPE; @@ -1155,40 +1148,36 @@ protected String getDeclaratorName(IResolvedOperation resolved) { protected void createExceptionMismatchError(IResolvedOperation operation, EObject sourceElement, List exceptionMismatch) { List exceptions = operation.getIllegallyDeclaredExceptions(); - StringBuilder message = new StringBuilder(100); - message.append("The declared exception"); - if (exceptions.size() > 1) { - message.append('s'); - } - message.append(' '); - for(int i = 0; i < exceptions.size(); i++) { - if (i != 0) { - if (i != exceptions.size() - 1) - message.append(", "); - else - message.append(" and "); - } - message.append(exceptions.get(i).getHumanReadableName()); - } - if (exceptions.size() > 1) { - message.append(" are"); - } else { - message.append(" is"); - } - message.append(" not compatible with throws clause in "); + + var suffixMessage = new StringBuilder(); + suffixMessage.append(" not compatible with throws clause in "); + for(int i = 0; i < exceptionMismatch.size(); i++) { if (i != 0) { if (i != exceptionMismatch.size() - 1) - message.append(", "); + suffixMessage.append(", "); else - message.append(" and "); + suffixMessage.append(" and "); } IResolvedOperation resolvedOperation = exceptionMismatch.get(i); - message.append(getDeclaratorName(resolvedOperation)); - message.append('.'); - message.append(exceptionMismatch.get(i).getSimpleSignature()); + suffixMessage.append(getDeclaratorName(resolvedOperation)); + suffixMessage.append('.'); + suffixMessage.append(exceptionMismatch.get(i).getSimpleSignature()); + } + + List resolvedExceptions = operation.getResolvedExceptions(); + for(int i = 0; i < exceptions.size(); i++) { + var message = new StringBuilder(100); + message.append("The declared exception "); + LightweightTypeReference exception = exceptions.get(i); + message.append(exception.getHumanReadableName()); + message.append(" is"); + message.append(suffixMessage); + var exceptionIndex = resolvedExceptions.indexOf(exception); + error(message.toString(), + sourceElement, XTEND_EXECUTABLE__EXCEPTIONS, + exceptionIndex, INCOMPATIBLE_THROWS_CLAUSE); } - error(message.toString(), sourceElement, exceptionsFeature(sourceElement), INCOMPATIBLE_THROWS_CLAUSE); } protected EObject findPrimarySourceElement(IResolvedOperation operation) { From 71ba175ca4644efa58106f362fc38ea2beebd4f1 Mon Sep 17 00:00:00 2001 From: Lorenzo Bettini Date: Thu, 18 Jan 2024 18:37:20 +0100 Subject: [PATCH 3/4] foreach instead of for --- .../org/eclipse/xtend/core/validation/XtendValidator.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/org.eclipse.xtend.core/src/org/eclipse/xtend/core/validation/XtendValidator.java b/org.eclipse.xtend.core/src/org/eclipse/xtend/core/validation/XtendValidator.java index 574bc2b9c06..35e42ebdd07 100644 --- a/org.eclipse.xtend.core/src/org/eclipse/xtend/core/validation/XtendValidator.java +++ b/org.eclipse.xtend.core/src/org/eclipse/xtend/core/validation/XtendValidator.java @@ -1152,7 +1152,7 @@ protected void createExceptionMismatchError(IResolvedOperation operation, EObjec var suffixMessage = new StringBuilder(); suffixMessage.append(" not compatible with throws clause in "); - for(int i = 0; i < exceptionMismatch.size(); i++) { + for (int i = 0; i < exceptionMismatch.size(); i++) { if (i != 0) { if (i != exceptionMismatch.size() - 1) suffixMessage.append(", "); @@ -1166,10 +1166,9 @@ protected void createExceptionMismatchError(IResolvedOperation operation, EObjec } List resolvedExceptions = operation.getResolvedExceptions(); - for(int i = 0; i < exceptions.size(); i++) { + for (LightweightTypeReference exception : exceptions) { var message = new StringBuilder(100); message.append("The declared exception "); - LightweightTypeReference exception = exceptions.get(i); message.append(exception.getHumanReadableName()); message.append(" is"); message.append(suffixMessage); From eff2e7b33406a479127f9e48cf0b81c37465c4a3 Mon Sep 17 00:00:00 2001 From: Lorenzo Bettini Date: Fri, 19 Jan 2024 11:41:01 +0100 Subject: [PATCH 4/4] some refinements --- .../core/tests/validation/OverrideValidationTest.java | 7 ++++--- .../org/eclipse/xtend/core/validation/XtendValidator.java | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/org.eclipse.xtend.core.tests/src/org/eclipse/xtend/core/tests/validation/OverrideValidationTest.java b/org.eclipse.xtend.core.tests/src/org/eclipse/xtend/core/tests/validation/OverrideValidationTest.java index 6ad7298ed7e..22b8516793f 100644 --- a/org.eclipse.xtend.core.tests/src/org/eclipse/xtend/core/tests/validation/OverrideValidationTest.java +++ b/org.eclipse.xtend.core.tests/src/org/eclipse/xtend/core/tests/validation/OverrideValidationTest.java @@ -514,15 +514,16 @@ public void testClassMustBeAbstract_06() throws Exception { } /** - * Three incompatible exceptions from different supertypes; - * the marker is set on the offending exception + * Two incompatible exceptions from three supertypes; + * the marker is set on the offending exceptions only. + * https://github.com/eclipse/xtext/issues/2912 */ @Test public void testIncompatibleThrowsClause_06() throws Exception { var source = "class Foo extends test.ExceptionThrowing implements test.ExceptionThrowingInterface, test.ExceptionThrowingInterface2 {" + "override nullPointerException() throws NullPointerException, java.io.IOException, java.io.FileNotFoundException {} " + "}"; XtendClass xtendClass = clazz(source); - var expectedSuffix = " is not compatible with throws clause in " + var expectedSuffix = " is not compatible with the throws clause in " + "ExceptionThrowing.nullPointerException(), ExceptionThrowingInterface.nullPointerException() and ExceptionThrowingInterface2.nullPointerException()"; helper.assertError(xtendClass.getMembers().get(0), XTEND_FUNCTION, INCOMPATIBLE_THROWS_CLAUSE, source.indexOf("java.io.IOException"), "java.io.IOException".length(), diff --git a/org.eclipse.xtend.core/src/org/eclipse/xtend/core/validation/XtendValidator.java b/org.eclipse.xtend.core/src/org/eclipse/xtend/core/validation/XtendValidator.java index 35e42ebdd07..56ac53702d9 100644 --- a/org.eclipse.xtend.core/src/org/eclipse/xtend/core/validation/XtendValidator.java +++ b/org.eclipse.xtend.core/src/org/eclipse/xtend/core/validation/XtendValidator.java @@ -1150,7 +1150,7 @@ protected void createExceptionMismatchError(IResolvedOperation operation, EObjec List exceptions = operation.getIllegallyDeclaredExceptions(); var suffixMessage = new StringBuilder(); - suffixMessage.append(" not compatible with throws clause in "); + suffixMessage.append(" not compatible with the throws clause in "); for (int i = 0; i < exceptionMismatch.size(); i++) { if (i != 0) {