-
Notifications
You must be signed in to change notification settings - Fork 330
Use destination URI when creating resource during copy refactoring #3588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,176 @@ | ||
| /** | ||
| * Copyright (c) 2026 TypeFox GmbH (http://www.typefox.io) 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 org.eclipse.xtext.ui.tests.refactoring; | ||
|
|
||
| import org.eclipse.core.resources.IContainer; | ||
| import org.eclipse.core.resources.IFile; | ||
| import org.eclipse.core.resources.IResource; | ||
| import org.eclipse.jdt.core.IJavaElement; | ||
| import org.eclipse.jdt.internal.corext.refactoring.reorg.IReorgPolicy.ICopyPolicy; | ||
| import org.eclipse.jdt.internal.corext.refactoring.reorg.ReorgDestinationFactory; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm internal imports ...
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Likely dep in manifest also missing.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There is, unfortunetaly, no generic copy processor for resources, so the test currently uses internal classes from JDT for that purpose. I could also create a mock copy processor to avoid the internal imports?
I have added the missing dependency for
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @szarnekow @LorenzoBettini i really dislike the new big bunch of internal deps but also have no better idea
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For tests it might acceptable. Otherwise, he suggested to use a mock avoiding the internal stuff |
||
| import org.eclipse.jdt.internal.corext.refactoring.reorg.ReorgPolicyFactory; | ||
| import org.eclipse.ltk.core.refactoring.RefactoringChangeDescriptor; | ||
| import org.eclipse.xtext.testing.InjectWith; | ||
| import org.eclipse.xtext.testing.XtextRunner; | ||
| import org.eclipse.xtext.testlanguages.fileAware.ui.tests.FileAwareTestLanguageUiInjectorProvider; | ||
| import org.junit.Assert; | ||
| import org.junit.Ignore; | ||
| import org.junit.Test; | ||
| import org.junit.runner.RunWith; | ||
|
|
||
| /** | ||
| * @author koehnlein - Initial contribution and API | ||
|
mx990 marked this conversation as resolved.
|
||
| * @author mx990 - adapted from ResourceMoveTest | ||
| */ | ||
| @InjectWith(FileAwareTestLanguageUiInjectorProvider.class) | ||
| @RunWith(XtextRunner.class) | ||
| public class ResourceCopyTest extends AbstractResourceRelocationTest { | ||
| @Test | ||
| public void testCopyFile() throws Exception { | ||
| String model1 = | ||
| "package foo.bar\n" + | ||
| "element X {\n" + | ||
| " ref X\n" + | ||
| "}\n"; | ||
| IFile x = file("foo/bar/X.fileawaretestlanguage", model1); | ||
| String model2 = | ||
| "package foo\n" + | ||
| "element Y {\n" + | ||
| " ref bar.X\n" + | ||
| "}\n"; | ||
| file("foo/Y.fileawaretestlanguage", model2); | ||
| performCopy(folder("foo/baz"), x); | ||
| Assert.assertTrue(x.exists()); | ||
| assertFileContents("foo/bar/X.fileawaretestlanguage", model1); | ||
| assertFileContents("foo/Y.fileawaretestlanguage", model2); | ||
| String expectation1 = | ||
| "package foo.baz\n" + | ||
| "element X {\n" + | ||
| " ref X\n" + | ||
| "}\n"; | ||
| assertFileContents("foo/baz/X.fileawaretestlanguage", expectation1); | ||
| } | ||
|
|
||
| @Test | ||
| public void testCopyFile_2() throws Exception { | ||
| String model1 = | ||
| "package foo.bar\n" + | ||
| "element X {\n" + | ||
| " ref X\n" + | ||
| "}\n"; | ||
| file("foo/bar/X.fileawaretestlanguage", model1); | ||
| String model2 = | ||
| "package foo\n" + | ||
| "element Y {\n" + | ||
| " ref bar.X\n" + | ||
| "}\n"; | ||
| IFile y = file("foo/Y.fileawaretestlanguage", model2); | ||
| performCopy(folder("foo/baz"), y); | ||
| Assert.assertTrue(y.exists()); | ||
| assertFileContents("foo/bar/X.fileawaretestlanguage", model1); | ||
| assertFileContents("foo/Y.fileawaretestlanguage", model2); | ||
| String expectation1 = | ||
| "package foo.baz\n" + | ||
| "element Y {\n" + | ||
| " ref foo.bar.X\n" + | ||
| "}\n"; | ||
| assertFileContents("foo/baz/Y.fileawaretestlanguage", expectation1); | ||
| } | ||
|
|
||
| @Test | ||
| public void testCopyFiles() throws Exception { | ||
| String model1 = | ||
| "package foo.bar\n" + | ||
| "element X {\n" + | ||
| " ref X\n" + | ||
| "}\n"; | ||
| IFile x = file("foo/X.fileawaretestlanguage", model1); | ||
| String model2 = | ||
| "package foo\n" + | ||
| "element Y {\n" + | ||
| " ref bar.X\n" + | ||
| "}\n"; | ||
| IFile y = file("foo/Y.fileawaretestlanguage", model2); | ||
| performCopy(folder("foo/baz"), x, y); | ||
| Assert.assertTrue(y.exists()); | ||
| assertFileContents("foo/X.fileawaretestlanguage", model1); | ||
| assertFileContents("foo/Y.fileawaretestlanguage", model2); | ||
| String expectation1 = | ||
| "package foo.baz\n" + | ||
| "element X {\n" + | ||
| " ref X\n" + | ||
| "}\n"; | ||
| assertFileContents("foo/baz/X.fileawaretestlanguage", expectation1); | ||
| String expectation2 = | ||
| "package foo.baz\n" + | ||
| "element Y {\n" + | ||
| " ref X\n" + | ||
| "}\n"; | ||
| assertFileContents("foo/baz/Y.fileawaretestlanguage", expectation2); | ||
| } | ||
|
|
||
| @Test | ||
| public void testCopyDirectory() throws Exception { | ||
| String model1 = | ||
| "package foo.bar\n" + | ||
| "element X {\n" + | ||
| " ref X\n" + | ||
| "}\n"; | ||
| IFile x = file("foo/bar/X.fileawaretestlanguage", model1); | ||
| String model2 = | ||
| "package foo\n" + | ||
| "element Y {\n" + | ||
| " ref bar.X\n" + | ||
| "}\n"; | ||
| file("foo/Y.fileawaretestlanguage", model2); | ||
| performCopy(folder("foo/baz"), x.getParent()); | ||
| Assert.assertTrue(x.exists()); | ||
| assertFileContents("foo/bar/X.fileawaretestlanguage", model1); | ||
| assertFileContents("foo/Y.fileawaretestlanguage", model2); | ||
| String expectation1 = | ||
| "package foo.baz.bar\n" + | ||
| "element X {\n" + | ||
| " ref X\n" + | ||
| "}\n"; | ||
| assertFileContents("foo/baz/bar/X.fileawaretestlanguage", expectation1); | ||
| } | ||
|
|
||
| @Test | ||
| public void testCopyDirectoryToRoot() throws Exception { | ||
| String model1 = | ||
| "package foo.bar\n" + | ||
| "element X {\n" + | ||
| " ref X\n" + | ||
| "}\n"; | ||
| IFile x = file("foo/bar/X.fileawaretestlanguage", model1); | ||
| String model2 = | ||
| "package foo\n" + | ||
| "element Y {\n" + | ||
| " ref bar.X\n" + | ||
| "}\n"; | ||
| file("foo/Y.fileawaretestlanguage", model2); | ||
| performCopy(project, x.getParent()); | ||
| Assert.assertTrue(x.exists()); | ||
| assertFileContents("foo/bar/X.fileawaretestlanguage", model1); | ||
| assertFileContents("foo/Y.fileawaretestlanguage", model2); | ||
| String expectation1 = | ||
| "package bar\n" + | ||
| "element X {\n" + | ||
| " ref X\n" + | ||
| "}\n"; | ||
| assertFileContents("bar/X.fileawaretestlanguage", expectation1); | ||
| } | ||
|
|
||
| @SuppressWarnings("restriction") | ||
| protected void performCopy(IContainer theDestination, IResource... theResources) throws Exception { | ||
| ICopyPolicy copyPolicy = ReorgPolicyFactory.createCopyPolicy(theResources, new IJavaElement[0]); | ||
| copyPolicy.setDestination(ReorgDestinationFactory.createDestination(theDestination)); | ||
| performRefactoring(((RefactoringChangeDescriptor) copyPolicy.getDescriptor()).getRefactoringDescriptor()); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wonder if this could me moved to ide tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tried to move the following related classes to
org.eclipse.xtext.ide.tests:AbstractResourceRelocationTest,ProgressReportingTest,ResourceCopyTest,ResourceMoveTest,ResourceRenameTest,but this would require the following additional dependencies:
org.eclipse.core.resources,org.eclipse.jdt.core,org.eclipse.jdt.core.manipulation,org.eclipse.ltk.core.refactoring,org.eclipse.xtext.testlanguages.ui,org.eclipse.xtext.ui.testing.Would you still like me to move the classes and add the additional dependencies?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i thought based on a lsp test or xtext.ide.tests this could be done without any of these dependencies....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(e.g. renameTestLanguage)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a quick look at the lsp server implementation and Xtext does not seem to support any copy resource refactoring via lsp, since it handles neither
willCreateFilesnordidCreateFilesfromorg.eclipse.lsp4j.services.WorkspaceService?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmmm. thats bad. is there really no way to avoid the new internal deps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned above, I could also create a mock copy processor to avoid the internal deps?