From 8bdbcc218fe8a6dfb2a78f0b3bb2ecb5f817ecc5 Mon Sep 17 00:00:00 2001 From: redcatbaer Date: Thu, 1 Oct 2026 21:22:04 +0200 Subject: [PATCH 1/4] #60: Fixed specification item ID renaming and validation. --- doc/changes/changes_0.11.0.md | 10 +- .../60-intellij-rename-refactoring.md | 32 +-- doc/design/building_block_view.md | 2 +- .../OftDeclarationNavigationElement.java | 23 ++ .../navigation/OftDeclarationResolver.java | 30 ++- .../navigation/OftRenameHandler.java | 168 ++++++++++++ .../navigation/OftRenameInputValidator.java | 45 ++++ .../OftRenamePsiElementProcessor.java | 69 +++-- src/main/resources/META-INF/plugin.xml | 2 + .../navigation/OftNavigationTest.java | 2 +- .../navigation/OftRenameHandlerTest.java | 242 ++++++++++++++++++ .../OftRenameInputValidatorTest.java | 94 +++++++ .../OftRenamePsiElementProcessorTest.java | 87 ++++++- 13 files changed, 752 insertions(+), 54 deletions(-) create mode 100644 src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandler.java create mode 100644 src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidator.java create mode 100644 src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandlerTest.java create mode 100644 src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidatorTest.java diff --git a/doc/changes/changes_0.11.0.md b/doc/changes/changes_0.11.0.md index eb8e923..e3fa4c8 100644 --- a/doc/changes/changes_0.11.0.md +++ b/doc/changes/changes_0.11.0.md @@ -1,11 +1,17 @@ -# OpenFastTrace IntelliJ Plugin 0.11.0, released 2026-09-22 +# OpenFastTrace IntelliJ Plugin 0.11.0, released 2026-10-01 -This maintenance release updates the bundled OpenFastTrace library and refreshes the Gradle build dependencies and plugins to current stable releases. +Version 0.11.0 introduces IntelliJ rename refactoring support for OpenFastTrace specification item IDs, updates the bundled OpenFastTrace library to 4.10.0, and refreshes the Gradle build dependencies and plugins to current stable versions. + +Users can trigger standard IntelliJ Rename refactoring (Shift+F6) directly on specification item ID declarations in supported specification documents. The rename dialog validates canonical OpenFastTrace identifier syntax (including tildes, hyphens, and dots), and matching references in `Covers:` sections and source coverage tags across the project update automatically. ## Bundled OpenFastTrace OpenFastTrace 4.10.0 +## Features + +* #60: Rename OpenFastTrace specification items with IntelliJ refactoring + ## Build Maintenance * Updated the Gradle wrapper, build plugins, JaCoCo, and test dependencies to current stable versions. diff --git a/doc/changesets/60-intellij-rename-refactoring.md b/doc/changesets/60-intellij-rename-refactoring.md index 212ce81..ea355f6 100644 --- a/doc/changesets/60-intellij-rename-refactoring.md +++ b/doc/changesets/60-intellij-rename-refactoring.md @@ -46,28 +46,28 @@ This keeps the implementation close to ordinary IntelliJ rename behavior: ### Requirements And Design -- [ ] Update `doc/system_requirements.md` with a feature requirement for OFT rename refactoring -- [ ] Add scenarios for renaming a declared OFT specification item and updating all resolved OFT references +- [x] Update `doc/system_requirements.md` with a feature requirement for OFT rename refactoring +- [x] Add scenarios for renaming a declared OFT specification item and updating all resolved OFT references - [ ] Stop and ask user for a review of the system requirements -- [ ] Update `doc/design/solution_strategy.md` to describe the IntelliJ-native rename approach -- [ ] Update `doc/design/building_block_view.md` and `doc/design/runtime_view.md` with the rename/refactoring flow +- [x] Update `doc/design/solution_strategy.md` to describe the IntelliJ-native rename approach +- [x] Update `doc/design/building_block_view.md` and `doc/design/runtime_view.md` with the rename/refactoring flow - [ ] Stop and ask user for a review of the design ### Implementation -- [ ] Make OFT specification item declarations participate in IntelliJ rename refactoring as the rename source -- [ ] Ensure OFT `Covers:` references and coverage tags resolve as rename usages for the renamed declaration -- [ ] Keep rename behavior limited to declarations that the plugin can map back to a canonical OFT item ID +- [x] Make OFT specification item declarations participate in IntelliJ rename refactoring as the rename source +- [x] Ensure OFT `Covers:` references and coverage tags resolve as rename usages for the renamed declaration +- [x] Keep rename behavior limited to declarations that the plugin can map back to a canonical OFT item ID ### Verification -- [ ] Add rename-refactoring tests that rename a specification declaration and verify the updated declaration text -- [ ] Add rename-refactoring tests that verify resolved `Covers:` references and coverage-tag usages are updated -- [ ] Add an index-and-navigation regression test that renames a specification item ID, confirms the new ID is searchable and navigable, and confirms the old entry no longer appears in index-driven navigation -- [ ] Add negative tests showing that non-declaration text does not participate in the rename flow -- [ ] Keep existing navigation, completion, and indexing tests green -- [ ] Keep the OpenFastTrace trace clean for the requirement and design artifact types in scope -- [ ] Keep required Gradle test, trace, packaging, and plugin verification tasks green +- [x] Add rename-refactoring tests that rename a specification declaration and verify the updated declaration text +- [x] Add rename-refactoring tests that verify resolved `Covers:` references and coverage-tag usages are updated +- [x] Add an index-and-navigation regression test that renames a specification item ID, confirms the new ID is searchable and navigable, and confirms the old entry no longer appears in index-driven navigation +- [x] Add negative tests showing that non-declaration text does not participate in the rename flow +- [x] Keep existing navigation, completion, and indexing tests green +- [x] Keep the OpenFastTrace trace clean for the requirement and design artifact types in scope +- [x] Keep required Gradle test, trace, packaging, and plugin verification tasks green - [ ] Keep SonarQube Cloud quality-gate checks green - [ ] Keep OSS Index audit results clean @@ -77,5 +77,5 @@ This keeps the implementation close to ordinary IntelliJ rename behavior: ## Version and Changelog Update -- [ ] Check whether this change should be part of a release version update or remain in the current unreleased line -- [ ] Write the changelog entry for the chosen release version if this issue is included in a release +- [x] Check whether this change should be part of a release version update or remain in the current unreleased line +- [x] Write the changelog entry for the chosen release version if this issue is included in a release diff --git a/doc/design/building_block_view.md b/doc/design/building_block_view.md index 57361e6..a7c96b7 100644 --- a/doc/design/building_block_view.md +++ b/doc/design/building_block_view.md @@ -296,7 +296,7 @@ Covers: - `scn~update-oft-references-after-rename~1` - `scn~show-renamed-oft-item-in-navigation~1` -Needs: impl +Needs: impl, itest ### User Guide Integration `dsn~user-guide-integration~1` diff --git a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftDeclarationNavigationElement.java b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftDeclarationNavigationElement.java index 17a44c1..32114f6 100644 --- a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftDeclarationNavigationElement.java +++ b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftDeclarationNavigationElement.java @@ -2,6 +2,7 @@ import com.intellij.openapi.fileEditor.OpenFileDescriptor; import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.io.FileUtil; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.PsiElement; @@ -28,6 +29,18 @@ final class OftDeclarationNavigationElement extends FakePsiElement { this.specification = specification; } + OftIndexedSpecification getSpecification() { + return specification; + } + + @Override + public TextRange getTextRange() { + if (specification != null) { + return new TextRange(specification.offset(), specification.offset() + specification.id().length()); + } + return super.getTextRange(); + } + @Override public PsiElement getParent() { return delegate == null ? null : delegate.getParent(); @@ -82,6 +95,16 @@ public boolean isValid() { return delegate != null && delegate.isValid(); } + @Override + public boolean isWritable() { + return delegate != null && delegate.isWritable(); + } + + @Override + public boolean isPhysical() { + return delegate != null && delegate.isPhysical(); + } + @Override public PsiManager getManager() { return requireDelegate().getManager(); diff --git a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftDeclarationResolver.java b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftDeclarationResolver.java index 71dcdfc..bd11f5f 100644 --- a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftDeclarationResolver.java +++ b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftDeclarationResolver.java @@ -6,6 +6,7 @@ import com.intellij.psi.PsiElement; import com.intellij.psi.PsiElementResolveResult; import com.intellij.psi.PsiFile; +import com.intellij.psi.PsiFileSystemItem; import com.intellij.psi.PsiManager; import com.intellij.psi.ResolveResult; import com.intellij.psi.search.SearchScope; @@ -42,26 +43,41 @@ static Optional findReferenceAt(final CharSequence text, f } static Optional findDeclaredItem(final PsiElement element) { - if (element == null || element.getContainingFile() == null) { + if (element == null || element instanceof PsiFileSystemItem) { + return Optional.empty(); + } + if (element instanceof OftDeclarationNavigationElement navElement) { + final OftIndexedSpecification spec = navElement.getSpecification(); + if (spec != null) { + return Optional.of(new OftSpecificationItem(spec.artifactType(), spec.name(), spec.revision())); + } + return Optional.empty(); + } + if (element.getContainingFile() == null) { return Optional.empty(); } final VirtualFile virtualFile = element.getContainingFile().getVirtualFile(); - if (virtualFile == null || !OftSupportedFiles.isSpecificationFile(virtualFile)) { + if (!OftSupportedFiles.isSpecificationFile(virtualFile)) { return Optional.empty(); } final TextRange textRange = element.getTextRange(); if (textRange == null) { return Optional.empty(); } - final int offset = textRange.getStartOffset(); - return OftSyntaxCore.findDefinitionSpecificationItems( - element.getContainingFile().getViewProvider().getContents() - ).stream() - .filter(match -> contains(match.span(), offset)) + final CharSequence fileText = element.getContainingFile().getViewProvider().getContents(); + return OftSyntaxCore.findDefinitionSpecificationItems(fileText).stream() + .filter(match -> isElementMatchingDeclarationSpan(textRange, match.span())) .map(OftSpecificationItemMatch::item) .findFirst(); } + private static boolean isElementMatchingDeclarationSpan(final TextRange textRange, final OftTextSpan span) { + final int start = textRange.getStartOffset(); + final int end = textRange.getEndOffset(); + return (contains(span, start) || (start >= span.startOffset() - 2 && end <= span.endOffset() + 2)) + && start >= span.startOffset() - 10; + } + private static Optional findCoverageTagReferenceAt( final CharSequence text, final int offset diff --git a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandler.java b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandler.java new file mode 100644 index 0000000..e546372 --- /dev/null +++ b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandler.java @@ -0,0 +1,168 @@ +package org.itsallcode.openfasttrace.intellijplugin.navigation; + +import com.intellij.openapi.actionSystem.CommonDataKeys; +import com.intellij.openapi.actionSystem.DataContext; +import com.intellij.openapi.editor.Editor; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.TextRange; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.psi.PsiDocumentManager; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiFile; +import com.intellij.refactoring.rename.PsiElementRenameHandler; +import com.intellij.refactoring.rename.RenameHandler; +import com.intellij.refactoring.util.CommonRefactoringUtil; +import org.itsallcode.openfasttrace.intellijplugin.OftSupportedFiles; +import org.itsallcode.openfasttrace.intellijplugin.indexing.OftIndexedSpecification; +import org.itsallcode.openfasttrace.intellijplugin.syntax.OftSpecificationItemMatch; +import org.itsallcode.openfasttrace.intellijplugin.syntax.OftSyntaxCore; +import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.Nullable; + +import java.util.Optional; + +// [impl->dsn~specification-item-rename~1] +public final class OftRenameHandler implements RenameHandler { + @Override + public boolean isAvailableOnDataContext(final @NonNull DataContext dataContext) { + final Editor editor = CommonDataKeys.EDITOR.getData(dataContext); + PsiFile file = CommonDataKeys.PSI_FILE.getData(dataContext); + if (file == null && editor != null) { + final Project project = CommonDataKeys.PROJECT.getData(dataContext); + if (project != null) { + file = PsiDocumentManager.getInstance(project).getPsiFile(editor.getDocument()); + } + } + if (editor != null && file != null) { + final VirtualFile virtualFile = file.getVirtualFile(); + if (virtualFile != null && OftSupportedFiles.isSpecificationFile(virtualFile)) { + final int offset = editor.getCaretModel().getOffset(); + if (findDeclarationAt(file, offset).isPresent()) { + return true; + } + } + } + final PsiElement element = CommonDataKeys.PSI_ELEMENT.getData(dataContext); + if (element != null && OftDeclarationResolver.findDeclaredItem(element).isPresent()) { + return true; + } + final PsiElement[] elements = CommonRefactoringUtil.getPsiElementArray(dataContext); + return elements.length == 1 && elements[0] != null + && OftDeclarationResolver.findDeclaredItem(elements[0]).isPresent(); + } + + @Override + public void invoke( + final @NonNull Project project, + final @Nullable Editor editor, + @Nullable PsiFile file, + final @NonNull DataContext dataContext + ) { + if (file == null && editor != null) { + file = PsiDocumentManager.getInstance(project).getPsiFile(editor.getDocument()); + } + if (editor != null && file != null) { + final int offset = editor.getCaretModel().getOffset(); + final Optional declaration = findDeclarationAt(file, offset); + if (declaration.isPresent()) { + performRename(project, editor, declaration.get(), dataContext); + return; + } + } + final PsiElement element = CommonDataKeys.PSI_ELEMENT.getData(dataContext); + if (element != null) { + resolveDeclarationTarget(element).ifPresent(target -> + performRename(project, editor, target, dataContext) + ); + return; + } + final PsiElement[] elements = CommonRefactoringUtil.getPsiElementArray(dataContext); + if (elements.length == 1 && elements[0] != null) { + resolveDeclarationTarget(elements[0]).ifPresent(target -> + performRename(project, editor, target, dataContext) + ); + } + } + + @Override + public void invoke( + final @NonNull Project project, + final PsiElement @NonNull [] elements, + final @NonNull DataContext dataContext + ) { + final Editor editor = CommonDataKeys.EDITOR.getData(dataContext); + if (elements.length == 1 && elements[0] != null) { + resolveDeclarationTarget(elements[0]).ifPresent(target -> + performRename(project, editor, target, dataContext) + ); + return; + } + final PsiElement element = CommonDataKeys.PSI_ELEMENT.getData(dataContext); + if (element != null) { + resolveDeclarationTarget(element).ifPresent(target -> + performRename(project, editor, target, dataContext) + ); + } + } + + private static void performRename( + final Project project, + final @Nullable Editor editor, + final PsiElement target, + final DataContext dataContext + ) { + final String defaultName = PsiElementRenameHandler.DEFAULT_NAME.getData(dataContext); + if (defaultName != null) { + PsiElementRenameHandler.rename(target, project, target, editor, defaultName); + } else { + PsiElementRenameHandler.rename(target, project, target, editor); + } + } + + static Optional findDeclarationAt( + final @Nullable PsiFile file, + final int offset + ) { + if (file == null || file.getVirtualFile() == null || !OftSupportedFiles.isSpecificationFile(file.getVirtualFile())) { + return Optional.empty(); + } + final CharSequence text = file.getViewProvider().getContents(); + for (OftSpecificationItemMatch match : OftSyntaxCore.findDefinitionSpecificationItems(text)) { + final int start = match.span().startOffset(); + final int end = match.span().endOffset(); + final int startBound = (start > 0 && text.charAt(start - 1) == '`') ? start - 1 : start; + final int endBound = (end < text.length() && text.charAt(end) == '`') ? end + 1 : end; + if (offset >= startBound && offset <= endBound) { + final PsiElement psiElement = file.findElementAt(start); + final OftIndexedSpecification spec = new OftIndexedSpecification( + match.item().artifactType(), + match.item().name(), + match.item().revision(), + start + ); + return Optional.of(new OftDeclarationNavigationElement( + psiElement != null ? psiElement : file, + spec + )); + } + } + return Optional.empty(); + } + + private static Optional resolveDeclarationTarget(final PsiElement element) { + if (element instanceof OftDeclarationNavigationElement) { + return Optional.of(element); + } + return OftDeclarationResolver.findDeclaredItem(element).map(declaredItem -> { + final TextRange range = element.getTextRange(); + final int offset = range != null ? range.getStartOffset() : element.getTextOffset(); + final OftIndexedSpecification spec = new OftIndexedSpecification( + declaredItem.artifactType(), + declaredItem.name(), + declaredItem.revision(), + offset + ); + return new OftDeclarationNavigationElement(element, spec); + }); + } +} diff --git a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidator.java b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidator.java new file mode 100644 index 0000000..f6d58fc --- /dev/null +++ b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidator.java @@ -0,0 +1,45 @@ +package org.itsallcode.openfasttrace.intellijplugin.navigation; + +import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.NlsContexts.DialogMessage; +import com.intellij.patterns.ElementPattern; +import com.intellij.patterns.PatternCondition; +import com.intellij.patterns.PlatformPatterns; +import com.intellij.psi.PsiElement; +import com.intellij.refactoring.rename.RenameInputValidatorEx; +import com.intellij.util.ProcessingContext; +import org.itsallcode.openfasttrace.intellijplugin.syntax.OftFragmentStatus; +import org.itsallcode.openfasttrace.intellijplugin.syntax.OftSyntaxCore; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +// [impl->dsn~specification-item-rename~1] +public final class OftRenameInputValidator implements RenameInputValidatorEx { + @Override + public @NotNull ElementPattern getPattern() { + return PlatformPatterns.psiElement().with(new PatternCondition<>("oftSpecificationDeclaration") { + @Override + public boolean accepts(final @NotNull PsiElement element, final ProcessingContext context) { + return element instanceof OftDeclarationNavigationElement + || OftDeclarationResolver.findDeclaredItem(element).isPresent(); + } + }); + } + + @Override + public boolean isInputValid( + final @NotNull String newName, + final @NotNull PsiElement element, + final @NotNull ProcessingContext context + ) { + return OftSyntaxCore.classifySpecificationItem(newName) == OftFragmentStatus.VALID; + } + + @Override + public @DialogMessage @Nullable String getErrorMessage(final @NotNull String newName, final @NotNull Project project) { + if (OftSyntaxCore.classifySpecificationItem(newName) == OftFragmentStatus.VALID) { + return null; + } + return "Specification item ID must have the form '~~', e.g. 'req~item-name~1'."; + } +} diff --git a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessor.java b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessor.java index 6f83a20..bfb7d92 100644 --- a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessor.java +++ b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessor.java @@ -6,6 +6,7 @@ import com.intellij.psi.PsiDocumentManager; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiFile; +import com.intellij.psi.PsiFileSystemItem; import com.intellij.psi.PsiManager; import com.intellij.psi.PsiReference; import com.intellij.refactoring.listeners.RefactoringElementListener; @@ -14,6 +15,7 @@ import com.intellij.util.IncorrectOperationException; import com.intellij.openapi.vfs.VirtualFile; import org.itsallcode.openfasttrace.intellijplugin.OftSupportedFiles; +import org.itsallcode.openfasttrace.intellijplugin.indexing.OftIndexedSpecification; import org.itsallcode.openfasttrace.intellijplugin.syntax.*; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; @@ -30,11 +32,17 @@ public final class OftRenamePsiElementProcessor extends RenamePsiElementProcessor { @Override public boolean canProcessElement(final @NonNull PsiElement element) { - return isSpecificationElement(element); + if (element instanceof PsiFileSystemItem) { + return false; + } + return OftDeclarationResolver.findDeclaredItem(element).isPresent(); } @Override public PsiElement substituteElementToRename(final @NonNull PsiElement element, final com.intellij.openapi.editor.Editor editor) { + if (element instanceof OftDeclarationNavigationElement) { + return element; + } if (editor == null || !isSpecificationElement(element)) { return element; } @@ -44,15 +52,30 @@ public PsiElement substituteElementToRename(final @NonNull PsiElement element, f } final int offset = editor.getCaretModel().getOffset(); final CharSequence fileText = element.getContainingFile().getViewProvider().getContents(); - return OftSyntaxCore.findDefinitionSpecificationItems(fileText).stream() - .filter(match -> offset >= match.span().startOffset() && offset < match.span().endOffset()) - .findFirst() - .map(match -> OftDeclarationResolver.findPsiElementAt( + for (OftSpecificationItemMatch match : OftSyntaxCore.findDefinitionSpecificationItems(fileText)) { + final int start = match.span().startOffset(); + final int end = match.span().endOffset(); + final int startBound = (start > 0 && fileText.charAt(start - 1) == '`') ? start - 1 : start; + final int endBound = (end < fileText.length() && fileText.charAt(end) == '`') ? end + 1 : end; + if (offset >= startBound && offset <= endBound) { + final PsiElement target = OftDeclarationResolver.findPsiElementAt( PsiManager.getInstance(element.getProject()), virtualFile, - match.span().startOffset() - )) - .orElse(element); + start + ); + final OftIndexedSpecification spec = new OftIndexedSpecification( + match.item().artifactType(), + match.item().name(), + match.item().revision(), + start + ); + return new OftDeclarationNavigationElement( + target != null ? target : element, + spec + ); + } + } + return element; } @Override @@ -62,19 +85,18 @@ public void renameElement( final UsageInfo @Nullable [] usages, final RefactoringElementListener listener ) throws IncorrectOperationException { - if (OftDeclarationResolver.findDeclaredItem(element).isEmpty()) { - throw new IncorrectOperationException("OpenFastTrace specification item declaration not found at rename target."); - } - if (OftSyntaxCore.classifySpecificationItem(newName) != OftFragmentStatus.VALID) { - throw new IncorrectOperationException("Invalid OpenFastTrace specification item ID: " + newName); - } - final String oldName = OftDeclarationResolver.findDeclaredItem(element) - .map(OftSpecificationItem::id) + final OftSpecificationItem oldItem = OftDeclarationResolver.findDeclaredItem(element) .orElseThrow(() -> new IncorrectOperationException( "OpenFastTrace specification item declaration not found at rename target." )); - replaceDeclarationText(element, oldName, newName); - updateProjectReferences(element.getProject(), oldName, newName); + if (OftSyntaxCore.classifySpecificationItem(newName) != OftFragmentStatus.VALID) { + throw new IncorrectOperationException("Invalid OpenFastTrace specification item ID: " + newName); + } + final Project project = element.getProject(); + final PsiFile psiFile = element.getContainingFile(); + final String oldName = oldItem.id(); + replaceDeclarationText(project, psiFile, oldName, newName); + updateProjectReferences(project, oldName, newName); if (usages != null) { Arrays.stream(usages) .map(UsageInfo::getReference) @@ -84,12 +106,15 @@ public void renameElement( } private static void replaceDeclarationText( - final PsiElement element, + final Project project, + final PsiFile psiFile, final String oldName, final String newText ) { - final PsiFile psiFile = element.getContainingFile(); - final Document document = PsiDocumentManager.getInstance(element.getProject()).getDocument(psiFile); + if (psiFile == null) { + return; + } + final Document document = PsiDocumentManager.getInstance(project).getDocument(psiFile); if (document == null) { return; } @@ -104,7 +129,7 @@ private static void replaceDeclarationText( for (OftTextSpan span : spans) { replaceSpan(document, span, newText); } - PsiDocumentManager.getInstance(element.getProject()).commitDocument(document); + PsiDocumentManager.getInstance(project).commitDocument(document); } private static void replaceReferenceText(final PsiReference reference, final String newText) { diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml index 93a0aa8..7d06e66 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -129,6 +129,8 @@ language="TEXT" implementation="org.itsallcode.openfasttrace.intellijplugin.navigation.OftCoverageTagReferenceContributor"/> + + diff --git a/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftNavigationTest.java b/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftNavigationTest.java index e068226..bff0d4b 100644 --- a/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftNavigationTest.java +++ b/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftNavigationTest.java @@ -455,7 +455,7 @@ public void testGivenSpecificationItemWhenRenamedThenTheDeclarationReferencesAnd EdtTestUtil.runInEdtAndWait(() -> WriteCommandAction.runWriteCommandAction( getProject(), () -> new OftRenamePsiElementProcessor().renameElement( - myFixture.getFile(), + declarationElementAtCaret(), "req~new_name~1", null, null diff --git a/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandlerTest.java b/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandlerTest.java new file mode 100644 index 0000000..029fa46 --- /dev/null +++ b/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandlerTest.java @@ -0,0 +1,242 @@ +package org.itsallcode.openfasttrace.intellijplugin.navigation; + +import com.intellij.openapi.actionSystem.CommonDataKeys; +import com.intellij.openapi.actionSystem.DataContext; +import com.intellij.openapi.actionSystem.PlatformCoreDataKeys; +import com.intellij.openapi.actionSystem.impl.SimpleDataContext; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiFile; +import com.intellij.refactoring.rename.PsiElementRenameHandler; +import com.intellij.testFramework.EdtTestUtil; +import org.itsallcode.openfasttrace.intellijplugin.AbstractOftPlatformTestCase; +import org.itsallcode.openfasttrace.intellijplugin.indexing.OftIndexedSpecification; + +import java.util.Objects; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.assertAll; + +// [itest->dsn~specification-item-rename~1] +public class OftRenameHandlerTest extends AbstractOftPlatformTestCase { + private final OftRenameHandler handler = new OftRenameHandler(); + + public void testGivenCaretOnSpecificationDeclarationWhenCheckingIsAvailableThenItReturnsTrue() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + myFixture.configureFromExistingVirtualFile(specFile.getVirtualFile()); + myFixture.getEditor().getCaretModel().moveToOffset(5); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.EDITOR, myFixture.getEditor()) + .add(CommonDataKeys.PSI_FILE, myFixture.getFile()) + .add(CommonDataKeys.PROJECT, getProject()) + .build(); + + assertThat(handler.isAvailableOnDataContext(dataContext), is(true)); + } + + public void testGivenCaretOnBacktickDeclarationWhenCheckingIsAvailableThenItReturnsTrue() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + `req~test_item~1` + Needs: dsn + """); + myFixture.configureFromExistingVirtualFile(specFile.getVirtualFile()); + myFixture.getEditor().getCaretModel().moveToOffset(6); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.EDITOR, myFixture.getEditor()) + .add(CommonDataKeys.PSI_FILE, myFixture.getFile()) + .add(CommonDataKeys.PROJECT, getProject()) + .build(); + + assertThat(handler.isAvailableOnDataContext(dataContext), is(true)); + } + + public void testGivenCaretAdjacentToBackticksWhenCheckingIsAvailableThenItReturnsTrue() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + `req~test_item~1` + Needs: dsn + """); + myFixture.configureFromExistingVirtualFile(specFile.getVirtualFile()); + + myFixture.getEditor().getCaretModel().moveToOffset(0); + final DataContext dataContextBeforeBacktick = SimpleDataContext.builder() + .add(CommonDataKeys.EDITOR, myFixture.getEditor()) + .add(CommonDataKeys.PSI_FILE, myFixture.getFile()) + .add(CommonDataKeys.PROJECT, getProject()) + .build(); + + myFixture.getEditor().getCaretModel().moveToOffset(17); + final DataContext dataContextAfterBacktick = SimpleDataContext.builder() + .add(CommonDataKeys.EDITOR, myFixture.getEditor()) + .add(CommonDataKeys.PSI_FILE, myFixture.getFile()) + .add(CommonDataKeys.PROJECT, getProject()) + .build(); + + assertAll( + () -> assertThat(handler.isAvailableOnDataContext(dataContextBeforeBacktick), is(true)), + () -> assertThat(handler.isAvailableOnDataContext(dataContextAfterBacktick), is(true)) + ); + } + + public void testGivenCaretOnNonDeclarationTextInMarkdownWhenCheckingIsAvailableThenItReturnsFalse() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + # Header Title + + req~test_item~1 + Needs: dsn + """); + myFixture.configureFromExistingVirtualFile(specFile.getVirtualFile()); + myFixture.getEditor().getCaretModel().moveToOffset(4); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.EDITOR, myFixture.getEditor()) + .add(CommonDataKeys.PSI_FILE, myFixture.getFile()) + .add(CommonDataKeys.PROJECT, getProject()) + .build(); + + assertThat(handler.isAvailableOnDataContext(dataContext), is(false)); + } + + public void testGivenCaretInNonSpecificationFileWhenCheckingIsAvailableThenItReturnsFalse() { + final PsiFile javaFile = myFixture.addFileToProject("src/Main.java", """ + // req~test_item~1 + class Main {} + """); + myFixture.configureFromExistingVirtualFile(javaFile.getVirtualFile()); + myFixture.getEditor().getCaretModel().moveToOffset(5); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.EDITOR, myFixture.getEditor()) + .add(CommonDataKeys.PSI_FILE, myFixture.getFile()) + .add(CommonDataKeys.PROJECT, getProject()) + .build(); + + assertThat(handler.isAvailableOnDataContext(dataContext), is(false)); + } + + public void testGivenEmptyDataContextWhenCheckingIsAvailableThenItReturnsFalse() { + assertThat(handler.isAvailableOnDataContext(DataContext.EMPTY_CONTEXT), is(false)); + } + + public void testGivenDataContextWithDeclarationElementWhenCheckingIsAvailableThenItReturnsTrue() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + final OftDeclarationNavigationElement navElement = new OftDeclarationNavigationElement( + declarationElement, + new OftIndexedSpecification("req", "test_item", 1, 0) + ); + + final DataContext dataContextWithPsiElement = SimpleDataContext.builder() + .add(CommonDataKeys.PSI_ELEMENT, navElement) + .add(CommonDataKeys.PROJECT, getProject()) + .build(); + + final DataContext dataContextWithPsiElementArray = SimpleDataContext.builder() + .add(PlatformCoreDataKeys.PSI_ELEMENT_ARRAY, new PsiElement[]{navElement}) + .add(CommonDataKeys.PROJECT, getProject()) + .build(); + + assertAll( + () -> assertThat(handler.isAvailableOnDataContext(dataContextWithPsiElement), is(true)), + () -> assertThat(handler.isAvailableOnDataContext(dataContextWithPsiElementArray), is(true)) + ); + } + + public void testGivenCaretOnDeclarationWhenInvokingRenameViaHandlerThenDeclarationAndReferencesAreUpdated() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + final PsiFile designFile = myFixture.addFileToProject("doc/design.md", """ + dsn~design_item~1 + Covers: + - req~test_item~1 + """); + final PsiFile javaFile = myFixture.addFileToProject("src/Main.java", + "// [" + "impl->req~test_item~1]\n" + + "class Main {}\n" + ); + + myFixture.configureFromExistingVirtualFile(specFile.getVirtualFile()); + myFixture.getEditor().getCaretModel().moveToOffset(4); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.EDITOR, myFixture.getEditor()) + .add(CommonDataKeys.PSI_FILE, myFixture.getFile()) + .add(CommonDataKeys.PROJECT, getProject()) + .add(PsiElementRenameHandler.DEFAULT_NAME, "req~renamed_item~2") + .build(); + + EdtTestUtil.runInEdtAndWait(() -> + handler.invoke(getProject(), myFixture.getEditor(), myFixture.getFile(), dataContext) + ); + + assertAll( + () -> assertThat(specFile.getText(), containsString("req~renamed_item~2")), + () -> assertThat(specFile.getText(), not(containsString("req~test_item~1"))), + () -> assertThat(designFile.getText(), containsString("req~renamed_item~2")), + () -> assertThat(javaFile.getText(), containsString("[" + "impl->req~renamed_item~2]")) + ); + } + + public void testGivenCaretOnDeclarationWhenRenamingAtCaretUsingHandlerThenRefactoringSucceeds() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + `feat~excuse_of_the_day~1` + Needs: dsn + """); + final PsiFile designFile = myFixture.addFileToProject("doc/design.md", """ + dsn~excuse_design~1 + Covers: + - feat~excuse_of_the_day~1 + """); + + myFixture.configureFromExistingVirtualFile(specFile.getVirtualFile()); + myFixture.getEditor().getCaretModel().moveToOffset(5); + + EdtTestUtil.runInEdtAndWait(() -> + myFixture.renameElementAtCaretUsingHandler("feat~better_excuse~2") + ); + + assertAll( + () -> assertThat(specFile.getText(), containsString("`feat~better_excuse~2`")), + () -> assertThat(specFile.getText(), not(containsString("feat~excuse_of_the_day~1"))), + () -> assertThat(designFile.getText(), containsString("feat~better_excuse~2")) + ); + } + + public void testGivenElementsArrayWhenInvokingRenameThenDeclarationAndReferencesAreUpdated() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + final PsiFile designFile = myFixture.addFileToProject("doc/design.md", """ + dsn~design_item~1 + Covers: + - req~test_item~1 + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.PROJECT, getProject()) + .add(PsiElementRenameHandler.DEFAULT_NAME, "req~renamed_item~2") + .build(); + + EdtTestUtil.runInEdtAndWait(() -> + handler.invoke(getProject(), new PsiElement[]{declarationElement}, dataContext) + ); + + assertAll( + () -> assertThat(specFile.getText(), containsString("req~renamed_item~2")), + () -> assertThat(designFile.getText(), containsString("req~renamed_item~2")) + ); + } +} diff --git a/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidatorTest.java b/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidatorTest.java new file mode 100644 index 0000000..a643693 --- /dev/null +++ b/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidatorTest.java @@ -0,0 +1,94 @@ +package org.itsallcode.openfasttrace.intellijplugin.navigation; + +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiFile; +import com.intellij.util.ProcessingContext; +import org.itsallcode.openfasttrace.intellijplugin.AbstractOftPlatformTestCase; +import org.itsallcode.openfasttrace.intellijplugin.indexing.OftIndexedSpecification; + +import java.util.Objects; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.jupiter.api.Assertions.assertAll; + +// [itest->dsn~specification-item-rename~1] +public class OftRenameInputValidatorTest extends AbstractOftPlatformTestCase { + private final OftRenameInputValidator validator = new OftRenameInputValidator(); + + public void testGivenValidSpecificationItemIdsWhenCheckingIsValidThenReturnsTrue() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test-item.name_1~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + final ProcessingContext context = new ProcessingContext(); + + assertAll( + () -> assertThat(validator.isInputValid("req~tell-late-work-excuse~1", declarationElement, context), is(true)), + () -> assertThat(validator.isInputValid("dsn~sub.item-2~1", declarationElement, context), is(true)), + () -> assertThat(validator.isInputValid("feat~excuse_of_the_day~1", declarationElement, context), is(true)), + () -> assertThat(validator.isInputValid("impl~core.module-task_a~99", declarationElement, context), is(true)) + ); + } + + public void testGivenInvalidSpecificationItemIdsWhenCheckingIsValidThenReturnsFalse() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test-item.name_1~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + final ProcessingContext context = new ProcessingContext(); + + assertAll( + () -> assertThat(validator.isInputValid("invalid_name", declarationElement, context), is(false)), + () -> assertThat(validator.isInputValid("req~missing_revision", declarationElement, context), is(false)), + () -> assertThat(validator.isInputValid("req~item~not_a_number", declarationElement, context), is(false)), + () -> assertThat(validator.isInputValid("~item~1", declarationElement, context), is(false)), + () -> assertThat(validator.isInputValid("123~item~1", declarationElement, context), is(false)), + () -> assertThat(validator.isInputValid("", declarationElement, context), is(false)) + ); + } + + public void testGivenValidAndInvalidIdsWhenGettingErrorMessageThenReturnsExpectedMessages() { + assertAll( + () -> assertThat(validator.getErrorMessage("req~valid-item~1", getProject()), is(nullValue())), + () -> assertThat( + validator.getErrorMessage("invalid_id", getProject()), + containsString("Specification item ID must have the form '~~'") + ) + ); + } + + public void testGivenSpecificationDeclarationElementWhenTestingPatternAcceptanceThenReturnsTrue() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + final OftDeclarationNavigationElement navElement = new OftDeclarationNavigationElement( + declarationElement, + new OftIndexedSpecification("req", "test_item", 1, 0) + ); + + final ProcessingContext context = new ProcessingContext(); + assertAll( + () -> assertThat(validator.getPattern().accepts(declarationElement, context), is(true)), + () -> assertThat(validator.getPattern().accepts(navElement, context), is(true)) + ); + } + + public void testGivenNonDeclarationElementWhenTestingPatternAcceptanceThenReturnsFalse() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + # Markdown Header + + Some regular body text. + """); + final PsiElement headerElement = Objects.requireNonNull(specFile.findElementAt(0)); + final ProcessingContext context = new ProcessingContext(); + + assertThat(validator.getPattern().accepts(headerElement, context), is(false)); + } +} diff --git a/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessorTest.java b/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessorTest.java index 099bc15..9ddb231 100644 --- a/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessorTest.java +++ b/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessorTest.java @@ -25,13 +25,63 @@ public class OftRenamePsiElementProcessorTest extends AbstractOftPlatformTestCase { private final OftRenamePsiElementProcessor processor = new OftRenamePsiElementProcessor(); - public void testGivenSpecificationElementWhenCheckingCanProcessThenItReturnsTrue() { + public void testGivenSpecificationFileWhenCheckingCanProcessThenItReturnsFalse() { final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ req~test_item~1 Needs: dsn """); - assertThat(processor.canProcessElement(specFile), is(true)); + assertThat(processor.canProcessElement(specFile), is(false)); + } + + public void testGivenDirectoryWhenCheckingCanProcessThenItReturnsFalse() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + + assertThat(processor.canProcessElement(Objects.requireNonNull(specFile.getParent())), is(false)); + } + + public void testGivenSpecificationDeclarationElementWhenCheckingCanProcessThenItReturnsTrue() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + + assertThat(processor.canProcessElement(declarationElement), is(true)); + } + + public void testGivenDeclarationNavigationElementWhenCheckingCanProcessThenItReturnsTrue() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + final OftDeclarationNavigationElement navigationElement = new OftDeclarationNavigationElement( + declarationElement, + new org.itsallcode.openfasttrace.intellijplugin.indexing.OftIndexedSpecification( + "req", + "test_item", + 1, + 0 + ) + ); + + assertThat(processor.canProcessElement(navigationElement), is(true)); + } + + public void testGivenNonDeclarationElementInSpecificationFileWhenCheckingCanProcessThenItReturnsFalse() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + Some leading documentation text. + + req~test_item~1 + Needs: dsn + """); + final PsiElement textElement = Objects.requireNonNull(specFile.findElementAt(5)); + + assertThat(processor.canProcessElement(textElement), is(false)); } public void testGivenNonSpecificationElementWhenCheckingCanProcessThenItReturnsFalse() { @@ -68,7 +118,7 @@ public void testGivenCaretOnSpecificationDefinitionWhenSubstitutingElementThenIt Assertions.assertAll( () -> assertThat(substituted, notNullValue()), - () -> assertThat(processor.canProcessElement(substituted), is(true)) + () -> assertThat(processor.canProcessElement(Objects.requireNonNull(substituted)), is(true)) ); } @@ -109,10 +159,11 @@ public void testGivenInvalidNewIdWhenRenamingThenItThrowsException() { Needs: dsn """); myFixture.configureFromExistingVirtualFile(specFile.getVirtualFile()); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); final IncorrectOperationException exception = Assertions.assertThrows( IncorrectOperationException.class, - () -> processor.renameElement(specFile, "invalid_specification_id", null, null) + () -> processor.renameElement(declarationElement, "invalid_specification_id", null, null) ); assertThat( @@ -137,6 +188,7 @@ public void testGivenUsagesAndNestedDirectoriesWhenRenamingThenAllReferencesAndT + "class Service {}\n" ); myFixture.configureFromExistingVirtualFile(specFile.getVirtualFile()); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); final OftSpecificationItem targetItem = new OftSpecificationItem("req", "rename_target", 1); final OftSpecificationIdReference reference = new OftSpecificationIdReference( @@ -151,7 +203,7 @@ public void testGivenUsagesAndNestedDirectoriesWhenRenamingThenAllReferencesAndT EdtTestUtil.runInEdtAndWait(() -> WriteCommandAction.runWriteCommandAction( getProject(), () -> processor.renameElement( - specFile, + declarationElement, "req~renamed_item~1", new UsageInfo[]{usageInfo}, null @@ -166,6 +218,31 @@ public void testGivenUsagesAndNestedDirectoriesWhenRenamingThenAllReferencesAndT ); } + public void testGivenFileRenameForMarkdownJavaAndTextFilesThenProcessorDoesNotInterfere() { + final PsiFile markdownFile = myFixture.addFileToProject("doc/spec.md", """ + req~rename_target~1 + Needs: dsn + """); + final PsiFile javaFile = myFixture.addFileToProject("src/Main.java", "class Main {}"); + final PsiFile textFile = myFixture.addFileToProject("doc/notes.txt", "Some plain notes."); + + Assertions.assertAll( + () -> assertThat(processor.canProcessElement(markdownFile), is(false)), + () -> assertThat(processor.canProcessElement(javaFile), is(false)), + () -> assertThat(processor.canProcessElement(textFile), is(false)) + ); + + myFixture.renameElement(markdownFile, "renamed-spec.md"); + myFixture.renameElement(javaFile, "RenamedMain.java"); + myFixture.renameElement(textFile, "renamed-notes.txt"); + + Assertions.assertAll( + () -> assertThat(markdownFile.getName(), is("renamed-spec.md")), + () -> assertThat(javaFile.getName(), is("RenamedMain.java")), + () -> assertThat(textFile.getName(), is("renamed-notes.txt")) + ); + } + public void testGivenUnrenameableReferenceWhenHandlingElementRenameThenItLeavesDocumentUnchanged() { final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ req~unrenameable_target~1 From 8ee8fc98bd0a98d7005e08451f4d4e5074809992 Mon Sep 17 00:00:00 2001 From: redcatbaer Date: Thu, 1 Oct 2026 21:31:40 +0200 Subject: [PATCH 2/4] #60: Fixed cyclomatic complexity. --- .../OftRenamePsiElementProcessor.java | 70 +++++++++++++------ 1 file changed, 49 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessor.java b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessor.java index bfb7d92..51c4f67 100644 --- a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessor.java +++ b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessor.java @@ -52,30 +52,58 @@ public PsiElement substituteElementToRename(final @NonNull PsiElement element, f } final int offset = editor.getCaretModel().getOffset(); final CharSequence fileText = element.getContainingFile().getViewProvider().getContents(); + final OftSpecificationItemMatch match = findMatchingSpecificationAtOffset(fileText, offset); + if (match != null) { + return createNavigationElement(element, virtualFile, match); + } + return element; + } + + private static @Nullable OftSpecificationItemMatch findMatchingSpecificationAtOffset( + final CharSequence fileText, + final int offset + ) { for (OftSpecificationItemMatch match : OftSyntaxCore.findDefinitionSpecificationItems(fileText)) { - final int start = match.span().startOffset(); - final int end = match.span().endOffset(); - final int startBound = (start > 0 && fileText.charAt(start - 1) == '`') ? start - 1 : start; - final int endBound = (end < fileText.length() && fileText.charAt(end) == '`') ? end + 1 : end; - if (offset >= startBound && offset <= endBound) { - final PsiElement target = OftDeclarationResolver.findPsiElementAt( - PsiManager.getInstance(element.getProject()), - virtualFile, - start - ); - final OftIndexedSpecification spec = new OftIndexedSpecification( - match.item().artifactType(), - match.item().name(), - match.item().revision(), - start - ); - return new OftDeclarationNavigationElement( - target != null ? target : element, - spec - ); + if (isOffsetWithinBounds(fileText, match.span(), offset)) { + return match; } } - return element; + return null; + } + + private static boolean isOffsetWithinBounds( + final CharSequence fileText, + final OftTextSpan span, + final int offset + ) { + final int start = span.startOffset(); + final int end = span.endOffset(); + final int startBound = (start > 0 && fileText.charAt(start - 1) == '`') ? start - 1 : start; + final int endBound = (end < fileText.length() && fileText.charAt(end) == '`') ? end + 1 : end; + return offset >= startBound && offset <= endBound; + } + + private static OftDeclarationNavigationElement createNavigationElement( + final PsiElement element, + final VirtualFile virtualFile, + final OftSpecificationItemMatch match + ) { + final int start = match.span().startOffset(); + final PsiElement target = OftDeclarationResolver.findPsiElementAt( + PsiManager.getInstance(element.getProject()), + virtualFile, + start + ); + final OftIndexedSpecification spec = new OftIndexedSpecification( + match.item().artifactType(), + match.item().name(), + match.item().revision(), + start + ); + return new OftDeclarationNavigationElement( + target != null ? target : element, + spec + ); } @Override From 8c79d0a000871907c70d29dc4065c18618fdb56c Mon Sep 17 00:00:00 2001 From: redcatbaer Date: Thu, 1 Oct 2026 22:12:34 +0200 Subject: [PATCH 3/4] #60: Fixed Sonar findings. --- .../navigation/OftRenameHandler.java | 122 ++++++---- .../navigation/OftRenameInputValidator.java | 5 +- .../OftRenamePsiElementProcessor.java | 42 ++-- .../navigation/OftRenameHandlerTest.java | 224 +++++++++++++++++- .../OftRenameInputValidatorTest.java | 30 +++ .../OftRenamePsiElementProcessorTest.java | 118 ++++++++- 6 files changed, 464 insertions(+), 77 deletions(-) diff --git a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandler.java b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandler.java index e546372..6aff85a 100644 --- a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandler.java +++ b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandler.java @@ -14,8 +14,10 @@ import com.intellij.refactoring.util.CommonRefactoringUtil; import org.itsallcode.openfasttrace.intellijplugin.OftSupportedFiles; import org.itsallcode.openfasttrace.intellijplugin.indexing.OftIndexedSpecification; +import org.itsallcode.openfasttrace.intellijplugin.syntax.OftSpecificationItem; import org.itsallcode.openfasttrace.intellijplugin.syntax.OftSpecificationItemMatch; import org.itsallcode.openfasttrace.intellijplugin.syntax.OftSyntaxCore; +import org.itsallcode.openfasttrace.intellijplugin.syntax.OftTextSpan; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; @@ -26,29 +28,38 @@ public final class OftRenameHandler implements RenameHandler { @Override public boolean isAvailableOnDataContext(final @NonNull DataContext dataContext) { final Editor editor = CommonDataKeys.EDITOR.getData(dataContext); + if (editor != null && isAvailableInEditor(editor, dataContext)) { + return true; + } + final PsiElement element = CommonDataKeys.PSI_ELEMENT.getData(dataContext); + if (hasDeclaredItem(element)) { + return true; + } + final PsiElement[] elements = CommonRefactoringUtil.getPsiElementArray(dataContext); + return elements.length == 1 && hasDeclaredItem(elements[0]); + } + + private static boolean isAvailableInEditor(final Editor editor, final DataContext dataContext) { PsiFile file = CommonDataKeys.PSI_FILE.getData(dataContext); - if (file == null && editor != null) { + if (file == null) { final Project project = CommonDataKeys.PROJECT.getData(dataContext); if (project != null) { file = PsiDocumentManager.getInstance(project).getPsiFile(editor.getDocument()); } } - if (editor != null && file != null) { - final VirtualFile virtualFile = file.getVirtualFile(); - if (virtualFile != null && OftSupportedFiles.isSpecificationFile(virtualFile)) { - final int offset = editor.getCaretModel().getOffset(); - if (findDeclarationAt(file, offset).isPresent()) { - return true; - } - } + if (file == null) { + return false; } - final PsiElement element = CommonDataKeys.PSI_ELEMENT.getData(dataContext); - if (element != null && OftDeclarationResolver.findDeclaredItem(element).isPresent()) { - return true; + final VirtualFile virtualFile = file.getVirtualFile(); + if (virtualFile == null || !OftSupportedFiles.isSpecificationFile(virtualFile)) { + return false; } - final PsiElement[] elements = CommonRefactoringUtil.getPsiElementArray(dataContext); - return elements.length == 1 && elements[0] != null - && OftDeclarationResolver.findDeclaredItem(elements[0]).isPresent(); + final int offset = editor.getCaretModel().getOffset(); + return findDeclarationAt(file, offset).isPresent(); + } + + private static boolean hasDeclaredItem(final @Nullable PsiElement element) { + return element != null && OftDeclarationResolver.findDeclaredItem(element).isPresent(); } @Override @@ -123,46 +134,69 @@ static Optional findDeclarationAt( final @Nullable PsiFile file, final int offset ) { - if (file == null || file.getVirtualFile() == null || !OftSupportedFiles.isSpecificationFile(file.getVirtualFile())) { + if (file == null || file.getVirtualFile() == null + || !OftSupportedFiles.isSpecificationFile(file.getVirtualFile())) { return Optional.empty(); } final CharSequence text = file.getViewProvider().getContents(); - for (OftSpecificationItemMatch match : OftSyntaxCore.findDefinitionSpecificationItems(text)) { - final int start = match.span().startOffset(); - final int end = match.span().endOffset(); - final int startBound = (start > 0 && text.charAt(start - 1) == '`') ? start - 1 : start; - final int endBound = (end < text.length() && text.charAt(end) == '`') ? end + 1 : end; - if (offset >= startBound && offset <= endBound) { - final PsiElement psiElement = file.findElementAt(start); - final OftIndexedSpecification spec = new OftIndexedSpecification( - match.item().artifactType(), - match.item().name(), - match.item().revision(), - start - ); - return Optional.of(new OftDeclarationNavigationElement( - psiElement != null ? psiElement : file, - spec - )); + for (final OftSpecificationItemMatch match : OftSyntaxCore.findDefinitionSpecificationItems(text)) { + if (isOffsetWithinBounds(text, match.span(), offset)) { + return Optional.of(createDeclarationElement(file, match)); } } return Optional.empty(); } + private static boolean isOffsetWithinBounds( + final CharSequence text, + final OftTextSpan span, + final int offset + ) { + final int start = span.startOffset(); + final int end = span.endOffset(); + final int startBound = ((start > 0) && (text.charAt(start - 1) == '`')) ? (start - 1) : start; + final int endBound = ((end < text.length()) && (text.charAt(end) == '`')) ? (end + 1) : end; + return (offset >= startBound) && (offset <= endBound); + } + + private static OftDeclarationNavigationElement createDeclarationElement( + final PsiFile file, + final OftSpecificationItemMatch match + ) { + final int start = match.span().startOffset(); + final PsiElement psiElement = file.findElementAt(start); + final OftIndexedSpecification spec = new OftIndexedSpecification( + match.item().artifactType(), + match.item().name(), + match.item().revision(), + start + ); + return new OftDeclarationNavigationElement( + psiElement != null ? psiElement : file, + spec + ); + } + private static Optional resolveDeclarationTarget(final PsiElement element) { if (element instanceof OftDeclarationNavigationElement) { return Optional.of(element); } - return OftDeclarationResolver.findDeclaredItem(element).map(declaredItem -> { - final TextRange range = element.getTextRange(); - final int offset = range != null ? range.getStartOffset() : element.getTextOffset(); - final OftIndexedSpecification spec = new OftIndexedSpecification( - declaredItem.artifactType(), - declaredItem.name(), - declaredItem.revision(), - offset - ); - return new OftDeclarationNavigationElement(element, spec); - }); + return OftDeclarationResolver.findDeclaredItem(element) + .map(declaredItem -> createDeclarationNavigationElement(element, declaredItem)); + } + + private static OftDeclarationNavigationElement createDeclarationNavigationElement( + final PsiElement element, + final OftSpecificationItem declaredItem + ) { + final TextRange range = element.getTextRange(); + final int offset = range != null ? range.getStartOffset() : element.getTextOffset(); + final OftIndexedSpecification spec = new OftIndexedSpecification( + declaredItem.artifactType(), + declaredItem.name(), + declaredItem.revision(), + offset + ); + return new OftDeclarationNavigationElement(element, spec); } } diff --git a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidator.java b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidator.java index f6d58fc..9e80c67 100644 --- a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidator.java +++ b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidator.java @@ -36,7 +36,10 @@ public boolean isInputValid( } @Override - public @DialogMessage @Nullable String getErrorMessage(final @NotNull String newName, final @NotNull Project project) { + public @DialogMessage @Nullable String getErrorMessage( + final @NotNull String newName, + final @NotNull Project project + ) { if (OftSyntaxCore.classifySpecificationItem(newName) == OftFragmentStatus.VALID) { return null; } diff --git a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessor.java b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessor.java index 51c4f67..46596f2 100644 --- a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessor.java +++ b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessor.java @@ -78,9 +78,9 @@ private static boolean isOffsetWithinBounds( ) { final int start = span.startOffset(); final int end = span.endOffset(); - final int startBound = (start > 0 && fileText.charAt(start - 1) == '`') ? start - 1 : start; - final int endBound = (end < fileText.length() && fileText.charAt(end) == '`') ? end + 1 : end; - return offset >= startBound && offset <= endBound; + final int startBound = ((start > 0) && (fileText.charAt(start - 1) == '`')) ? (start - 1) : start; + final int endBound = ((end < fileText.length()) && (fileText.charAt(end) == '`')) ? (end + 1) : end; + return (offset >= startBound) && (offset <= endBound); } private static OftDeclarationNavigationElement createNavigationElement( @@ -113,13 +113,13 @@ public void renameElement( final UsageInfo @Nullable [] usages, final RefactoringElementListener listener ) throws IncorrectOperationException { + if (OftSyntaxCore.classifySpecificationItem(newName) != OftFragmentStatus.VALID) { + throw new IncorrectOperationException("Invalid OpenFastTrace specification item ID: " + newName); + } final OftSpecificationItem oldItem = OftDeclarationResolver.findDeclaredItem(element) .orElseThrow(() -> new IncorrectOperationException( "OpenFastTrace specification item declaration not found at rename target." )); - if (OftSyntaxCore.classifySpecificationItem(newName) != OftFragmentStatus.VALID) { - throw new IncorrectOperationException("Invalid OpenFastTrace specification item ID: " + newName); - } final Project project = element.getProject(); final PsiFile psiFile = element.getContainingFile(); final String oldName = oldItem.id(); @@ -147,14 +147,12 @@ private static void replaceDeclarationText( return; } final CharSequence fileText = psiFile.getViewProvider().getContents(); - final List spans = new ArrayList<>(); - for (OftSpecificationItemMatch match : OftSyntaxCore.findDefinitionSpecificationItems(fileText)) { - if (oldName.equals(match.item().id())) { - spans.add(match.span()); - } - } - spans.sort(Comparator.comparingInt(OftTextSpan::startOffset).reversed()); - for (OftTextSpan span : spans) { + final List spans = OftSyntaxCore.findDefinitionSpecificationItems(fileText).stream() + .filter(match -> oldName.equals(match.item().id())) + .map(OftSpecificationItemMatch::span) + .sorted(Comparator.comparingInt(OftTextSpan::startOffset).reversed()) + .toList(); + for (final OftTextSpan span : spans) { replaceSpan(document, span, newText); } PsiDocumentManager.getInstance(project).commitDocument(document); @@ -208,16 +206,14 @@ private static void updateSpecificationFile( if (document == null) { return; } - final List spans = new ArrayList<>(); - for (OftSpecificationItemMatch match : OftDeclarationResolver.findCoveredSpecificationItems( + final List spans = OftDeclarationResolver.findCoveredSpecificationItems( psiFile.getViewProvider().getContents() - )) { - if (oldName.equals(match.item().id())) { - spans.add(match.span()); - } - } - spans.sort(Comparator.comparingInt(OftTextSpan::startOffset).reversed()); - for (OftTextSpan span : spans) { + ).stream() + .filter(match -> oldName.equals(match.item().id())) + .map(OftSpecificationItemMatch::span) + .sorted(Comparator.comparingInt(OftTextSpan::startOffset).reversed()) + .toList(); + for (final OftTextSpan span : spans) { replaceSpan(document, span, newName); } PsiDocumentManager.getInstance(project).commitDocument(document); diff --git a/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandlerTest.java b/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandlerTest.java index 029fa46..f3ad748 100644 --- a/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandlerTest.java +++ b/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandlerTest.java @@ -18,6 +18,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; // [itest->dsn~specification-item-rename~1] public class OftRenameHandlerTest extends AbstractOftPlatformTestCase { @@ -161,10 +162,11 @@ public void testGivenCaretOnDeclarationWhenInvokingRenameViaHandlerThenDeclarati Covers: - req~test_item~1 """); - final PsiFile javaFile = myFixture.addFileToProject("src/Main.java", - "// [" + "impl->req~test_item~1]\n" - + "class Main {}\n" - ); + final String coverageTag = "[impl" + "->req~test_item~1]"; + final PsiFile javaFile = myFixture.addFileToProject("src/Main.java", """ + // %s + class Main {} + """.formatted(coverageTag)); myFixture.configureFromExistingVirtualFile(specFile.getVirtualFile()); myFixture.getEditor().getCaretModel().moveToOffset(4); @@ -239,4 +241,218 @@ public void testGivenElementsArrayWhenInvokingRenameThenDeclarationAndReferences () -> assertThat(designFile.getText(), containsString("req~renamed_item~2")) ); } + + public void testGivenEditorWithoutPsiFileInContextWhenCheckingIsAvailableThenResolvesFileFromDocument() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + myFixture.configureFromExistingVirtualFile(specFile.getVirtualFile()); + myFixture.getEditor().getCaretModel().moveToOffset(5); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.EDITOR, myFixture.getEditor()) + .add(CommonDataKeys.PROJECT, getProject()) + .build(); + + assertThat(handler.isAvailableOnDataContext(dataContext), is(true)); + } + + public void testGivenEditorWithNullProjectAndNoDeclarationInDataContextWhenCheckingIsAvailableThenReturnsFalse() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + # Header + req~test_item~1 + """); + myFixture.configureFromExistingVirtualFile(specFile.getVirtualFile()); + myFixture.getEditor().getCaretModel().moveToOffset(2); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.EDITOR, myFixture.getEditor()) + .build(); + + assertThat(handler.isAvailableOnDataContext(dataContext), is(false)); + } + + public void testGivenNoDefaultNameInDataContextWhenInvokingRenameThenRenameWithoutDefaultNameIsCalled() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.PSI_ELEMENT, declarationElement) + .add(CommonDataKeys.PROJECT, getProject()) + .build(); + + assertDoesNotThrow(() -> EdtTestUtil.runInEdtAndWait(() -> + handler.invoke(getProject(), null, null, dataContext))); + } + + public void testGivenNoEditorAndPsiElementInContextWhenInvokingRenameThenPerformsRename() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + final PsiFile designFile = myFixture.addFileToProject("doc/design.md", """ + dsn~design_item~1 + Covers: + - req~test_item~1 + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + final OftDeclarationNavigationElement navElement = new OftDeclarationNavigationElement( + declarationElement, + new OftIndexedSpecification("req", "test_item", 1, 0) + ); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.PSI_ELEMENT, navElement) + .add(CommonDataKeys.PROJECT, getProject()) + .add(PsiElementRenameHandler.DEFAULT_NAME, "req~renamed_item~2") + .build(); + + EdtTestUtil.runInEdtAndWait(() -> + handler.invoke(getProject(), null, null, dataContext) + ); + + assertAll( + () -> assertThat(specFile.getText(), containsString("req~renamed_item~2")), + () -> assertThat(designFile.getText(), containsString("req~renamed_item~2")) + ); + } + + public void testGivenNoEditorAndRawPsiElementWithDeclaredItemWhenInvokingRenameThenPerformsRename() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.PSI_ELEMENT, declarationElement) + .add(CommonDataKeys.PROJECT, getProject()) + .add(PsiElementRenameHandler.DEFAULT_NAME, "req~renamed_item~2") + .build(); + + EdtTestUtil.runInEdtAndWait(() -> + handler.invoke(getProject(), null, null, dataContext) + ); + + assertThat(specFile.getText(), containsString("req~renamed_item~2")); + } + + public void testGivenNoEditorAndPsiElementArrayInContextWhenInvokingRenameThenPerformsRename() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + + final DataContext dataContext = SimpleDataContext.builder() + .add(PlatformCoreDataKeys.PSI_ELEMENT_ARRAY, new PsiElement[]{declarationElement}) + .add(CommonDataKeys.PROJECT, getProject()) + .add(PsiElementRenameHandler.DEFAULT_NAME, "req~renamed_item~2") + .build(); + + EdtTestUtil.runInEdtAndWait(() -> + handler.invoke(getProject(), null, null, dataContext) + ); + + assertThat(specFile.getText(), containsString("req~renamed_item~2")); + } + + public void testGivenEditorAndNullFileWhenInvokingRenameThenResolvesFileFromEditor() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + myFixture.configureFromExistingVirtualFile(specFile.getVirtualFile()); + myFixture.getEditor().getCaretModel().moveToOffset(4); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.EDITOR, myFixture.getEditor()) + .add(CommonDataKeys.PROJECT, getProject()) + .add(PsiElementRenameHandler.DEFAULT_NAME, "req~renamed_item~2") + .build(); + + EdtTestUtil.runInEdtAndWait(() -> + handler.invoke(getProject(), myFixture.getEditor(), null, dataContext) + ); + + assertThat(specFile.getText(), containsString("req~renamed_item~2")); + } + + public void testGivenCaretNotOnDeclarationAndNoPsiElementInContextWhenInvokingRenameThenDoesNothing() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + # Header + req~test_item~1 + Needs: dsn + """); + myFixture.configureFromExistingVirtualFile(specFile.getVirtualFile()); + myFixture.getEditor().getCaretModel().moveToOffset(2); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.EDITOR, myFixture.getEditor()) + .add(CommonDataKeys.PSI_FILE, myFixture.getFile()) + .add(CommonDataKeys.PROJECT, getProject()) + .add(PsiElementRenameHandler.DEFAULT_NAME, "req~renamed_item~2") + .build(); + + EdtTestUtil.runInEdtAndWait(() -> + handler.invoke(getProject(), myFixture.getEditor(), myFixture.getFile(), dataContext) + ); + + assertThat(specFile.getText(), containsString("req~test_item~1")); + } + + public void testGivenElementsArrayWithMultipleElementsWhenInvokingRenameThenFallsBackToDataContext() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.PSI_ELEMENT, declarationElement) + .add(CommonDataKeys.PROJECT, getProject()) + .add(PsiElementRenameHandler.DEFAULT_NAME, "req~renamed_item~2") + .build(); + + EdtTestUtil.runInEdtAndWait(() -> + handler.invoke(getProject(), new PsiElement[0], dataContext) + ); + + assertThat(specFile.getText(), containsString("req~renamed_item~2")); + } + + public void testGivenElementsArrayWithNavigationElementWhenInvokingRenameThenPerformsRename() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + final OftDeclarationNavigationElement navElement = new OftDeclarationNavigationElement( + declarationElement, + new OftIndexedSpecification("req", "test_item", 1, 0) + ); + + final DataContext dataContext = SimpleDataContext.builder() + .add(CommonDataKeys.PROJECT, getProject()) + .add(PsiElementRenameHandler.DEFAULT_NAME, "req~renamed_item~2") + .build(); + + EdtTestUtil.runInEdtAndWait(() -> + handler.invoke(getProject(), new PsiElement[]{navElement}, dataContext) + ); + + assertThat(specFile.getText(), containsString("req~renamed_item~2")); + } + + public void testFindDeclarationAtGivenNullOrNonSpecificationFileThenReturnsEmpty() { + final PsiFile javaFile = myFixture.addFileToProject("src/Main.java", "class Main {}"); + assertAll( + () -> assertThat(OftRenameHandler.findDeclarationAt(null, 0).isEmpty(), is(true)), + () -> assertThat(OftRenameHandler.findDeclarationAt(javaFile, 0).isEmpty(), is(true)) + ); + } } diff --git a/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidatorTest.java b/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidatorTest.java index a643693..e130e09 100644 --- a/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidatorTest.java +++ b/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameInputValidatorTest.java @@ -91,4 +91,34 @@ public void testGivenNonDeclarationElementWhenTestingPatternAcceptanceThenReturn assertThat(validator.getPattern().accepts(headerElement, context), is(false)); } + + public void testGivenNonMatchingPsiElementsWhenTestingPatternAcceptanceThenReturnsFalse() { + final PsiFile javaFile = myFixture.addFileToProject("src/Main.java", "class Main {}"); + final ProcessingContext context = new ProcessingContext(); + + assertAll( + () -> assertThat(validator.getPattern().accepts(javaFile, context), is(false)), + () -> assertThat( + validator.getPattern().accepts(Objects.requireNonNull(javaFile.findElementAt(0)), context), + is(false) + ) + ); + } + + public void testGivenVariousMalformedSpecificationItemIdsWhenCheckingIsValidThenReturnsFalse() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test-item.name_1~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + final ProcessingContext context = new ProcessingContext(); + + assertAll( + () -> assertThat(validator.isInputValid("req~~1", declarationElement, context), is(false)), + () -> assertThat(validator.isInputValid("req~name~", declarationElement, context), is(false)), + () -> assertThat(validator.isInputValid("req~name~-1", declarationElement, context), is(false)), + () -> assertThat(validator.isInputValid("unknown_type~name~1", declarationElement, context), is(false)), + () -> assertThat(validator.isInputValid(" ", declarationElement, context), is(false)) + ); + } } diff --git a/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessorTest.java b/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessorTest.java index 9ddb231..656d94a 100644 --- a/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessorTest.java +++ b/src/test/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenamePsiElementProcessorTest.java @@ -3,9 +3,11 @@ import com.intellij.openapi.command.WriteCommandAction; import com.intellij.openapi.editor.Document; import com.intellij.openapi.fileEditor.FileDocumentManager; +import com.intellij.openapi.fileTypes.PlainTextFileType; import com.intellij.openapi.util.TextRange; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiFile; +import com.intellij.psi.PsiFileFactory; import com.intellij.testFramework.EdtTestUtil; import com.intellij.usageView.UsageInfo; import com.intellij.util.IncorrectOperationException; @@ -182,11 +184,13 @@ public void testGivenUsagesAndNestedDirectoriesWhenRenamingThenAllReferencesAndT Covers: - req~rename_target~1 """); - final PsiFile javaFile = myFixture.addFileToProject("src/sub/pkg/Service.java", - "// " + "[" + "req~rename_target~1->req~other~1]\n" - + "// " + "[" + "impl~service~1->req~rename_target~1]\n" - + "class Service {}\n" - ); + final String tag1 = "[impl" + "~service~1->req~rename_target~1]"; + final String tag2 = "[" + "req~rename_target~1->req~other~1]"; + final PsiFile javaFile = myFixture.addFileToProject("src/sub/pkg/Service.java", """ + // %s + // %s + class Service {} + """.formatted(tag2, tag1)); myFixture.configureFromExistingVirtualFile(specFile.getVirtualFile()); final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); @@ -285,6 +289,110 @@ public void testGivenRenameableReferenceWhenHandlingElementRenameThenItUpdatesDo assertThat(documentText(specFile), containsString("req~renamed_item~1")); } + public void testGivenDeclarationNavigationElementWhenSubstitutingElementThenReturnsSameInstance() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~test_item~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + final OftDeclarationNavigationElement navElement = new OftDeclarationNavigationElement( + declarationElement, + new org.itsallcode.openfasttrace.intellijplugin.indexing.OftIndexedSpecification( + "req", "test_item", 1, 0 + ) + ); + + final PsiElement substituted = processor.substituteElementToRename(navElement, myFixture.getEditor()); + + assertThat(substituted, sameInstance(navElement)); + } + + public void testGivenInMemorySpecificationFileWithoutVirtualFileWhenSubstitutingElementThenReturnsOriginalElement() { + final PsiFile inMemorySpec = PsiFileFactory.getInstance(getProject()) + .createFileFromText("spec.md", PlainTextFileType.INSTANCE, "req~test~1\n"); + final PsiElement substituted = processor.substituteElementToRename(inMemorySpec, myFixture.getEditor()); + assertThat(substituted, sameInstance(inMemorySpec)); + } + + public void testGivenNullUsagesArrayWhenRenamingThenRefactoringSucceeds() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~null_usages_target~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + + EdtTestUtil.runInEdtAndWait(() -> WriteCommandAction.runWriteCommandAction( + getProject(), + () -> processor.renameElement(declarationElement, "req~renamed_null_usages~1", null, null) + )); + + assertThat(documentText(specFile), containsString("req~renamed_null_usages~1")); + } + + public void testGivenUsagesArrayWithNonNullAndNullReferencesWhenRenamingThenOnlyNonNullReferencesAreHandled() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~multi_usage_target~1 + Needs: dsn + """); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + + final OftSpecificationItem targetItem = new OftSpecificationItem("req", "multi_usage_target", 1); + final OftSpecificationIdReference reference = new OftSpecificationIdReference( + specFile, + new TextRange(0, "req~multi_usage_target~1".length()), + targetItem, + true + ); + final UsageInfo usageWithRef = new UsageInfo(specFile) { + @Override + public com.intellij.psi.PsiReference getReference() { + return reference; + } + }; + final UsageInfo usageWithoutRef = new UsageInfo(declarationElement); + + EdtTestUtil.runInEdtAndWait(() -> WriteCommandAction.runWriteCommandAction( + getProject(), + () -> processor.renameElement( + declarationElement, + "req~renamed_multi_usage~1", + new UsageInfo[]{usageWithRef, usageWithoutRef}, + null + ) + )); + + assertThat(documentText(specFile), containsString("req~renamed_multi_usage~1")); + } + + public void testGivenCoverageTagsWithFullIdSourceAndShortSourceWhenRenamingThenOnlyMatchingFullIdSourceIsRenamed() { + final PsiFile specFile = myFixture.addFileToProject("doc/spec.md", """ + req~full_id_source~1 + Needs: dsn + """); + final String tagA = "[" + "req~full_id_source~1->dsn~target~1]"; + final String tagB = "[impl" + "->req~full_id_source~1]"; + final String tagC = "[" + "req->req~other_target~1]"; + final PsiFile javaFile = myFixture.addFileToProject("src/Source.java", """ + // %s + // %s + // %s + class Source {} + """.formatted(tagA, tagB, tagC)); + final PsiElement declarationElement = Objects.requireNonNull(specFile.findElementAt(0)); + + EdtTestUtil.runInEdtAndWait(() -> WriteCommandAction.runWriteCommandAction( + getProject(), + () -> processor.renameElement(declarationElement, "req~renamed_source~2", null, null) + )); + + Assertions.assertAll( + () -> assertThat(documentText(specFile), containsString("req~renamed_source~2")), + () -> assertThat(documentText(javaFile), containsString("[" + "req~renamed_source~2->dsn~target~1]")), + () -> assertThat(documentText(javaFile), containsString("[" + "impl->req~renamed_source~2]")), + () -> assertThat(documentText(javaFile), containsString("[" + "req->req~other_target~1]")) + ); + } + private String documentText(final PsiFile file) { final Document document = FileDocumentManager.getInstance().getDocument(file.getVirtualFile()); if (document == null) { From 06144c10b3a2b0b67f114ba8be6e8c30cc6f1e93 Mon Sep 17 00:00:00 2001 From: redcatbaer Date: Thu, 1 Oct 2026 22:15:53 +0200 Subject: [PATCH 4/4] #60: Fixed IntelliJ finding. --- .../intellijplugin/navigation/OftRenameHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandler.java b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandler.java index 6aff85a..20c47d1 100644 --- a/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandler.java +++ b/src/main/java/org/itsallcode/openfasttrace/intellijplugin/navigation/OftRenameHandler.java @@ -51,7 +51,7 @@ private static boolean isAvailableInEditor(final Editor editor, final DataContex return false; } final VirtualFile virtualFile = file.getVirtualFile(); - if (virtualFile == null || !OftSupportedFiles.isSpecificationFile(virtualFile)) { + if (!OftSupportedFiles.isSpecificationFile(virtualFile)) { return false; } final int offset = editor.getCaretModel().getOffset();