[grazie] join adjacent comment texts with \n, not a space (IDEA-279919: unpaired symbol false positives)

GitOrigin-RevId: 1c42e091f56391f7b3642701e3490c92d43ca8b9
This commit is contained in:
Peter Gromov
2021-10-12 16:18:13 +02:00
committed by intellij-monorepo-bot
parent b3ac7c53f3
commit edc74347d1
12 changed files with 67 additions and 27 deletions

View File

@@ -49,7 +49,7 @@ public class JavaTextExtractor extends TextExtractor {
if (root instanceof PsiCommentImpl && allowedDomains.contains(COMMENTS)) {
List<PsiElement> roots = PsiUtilsKt.getNotSoDistantSimilarSiblings(root, e ->
JAVA_PLAIN_COMMENT_BIT_SET.contains(PsiUtilCore.getElementType(e)));
return TextContent.joinWithWhitespace(ContainerUtil.mapNotNull(roots, c ->
return TextContent.joinWithWhitespace('\n', ContainerUtil.mapNotNull(roots, c ->
TextContentBuilder.FromPsi.removingIndents(" \t*/").build(c, COMMENTS)));
}

View File

@@ -31,7 +31,7 @@ public class PropertyTextExtractor extends TextExtractor {
if (root instanceof PsiComment) {
List<PsiElement> roots = PsiUtilsKt.getNotSoDistantSimilarSiblings(root, e ->
PropertiesTokenTypes.COMMENTS.contains(PsiUtilCore.getElementType(e)));
return TextContent.joinWithWhitespace(ContainerUtil.mapNotNull(roots, c ->
return TextContent.joinWithWhitespace('\n', ContainerUtil.mapNotNull(roots, c ->
TextContentBuilder.FromPsi.removingIndents(" \t#!").build(c, COMMENTS)));
}
if (PsiUtilCore.getElementType(root) == PropertiesTokenTypes.VALUE_CHARACTERS) {

View File

@@ -18,7 +18,6 @@ import com.intellij.openapi.editor.Editor
import com.intellij.openapi.project.Project
import com.intellij.openapi.util.NlsSafe
import com.intellij.openapi.util.TextRange
import com.intellij.openapi.util.text.StringUtil
import com.intellij.psi.PsiFile
import com.intellij.psi.SmartPointerManager
import com.intellij.psi.SmartPsiFileRange
@@ -75,9 +74,9 @@ internal object GrazieReplaceTypoQuickFix {
val familyName = GrazieBundle.message("grazie.grammar.quickfix.replace.typo.text", problem.shortMessage)
val result = arrayListOf<LocalQuickFix>(ReplaceTypoTitleAction(familyName, problem.shortMessage))
problem.corrections.forEachIndexed { index, suggestion ->
val commonPrefix = StringUtil.commonPrefixLength(suggestion, replacedText)
val commonPrefix = commonPrefixLength(suggestion, replacedText)
val commonSuffix =
min(StringUtil.commonSuffixLength(suggestion, replacedText), min(suggestion.length, replacementRange.length) - commonPrefix)
min(commonSuffixLength(suggestion, replacedText), min(suggestion.length, replacementRange.length) - commonPrefix)
val localRange = TextRange(replacementRange.startOffset + commonPrefix, replacementRange.endOffset - commonSuffix)
val replacement = suggestion.substring(commonPrefix, suggestion.length - commonSuffix)
result.add(ChangeToVariantAction(
@@ -86,4 +85,23 @@ internal object GrazieReplaceTypoQuickFix {
}
return result
}
// custom common prefix/suffix calculation to honor cases when the text is separated by a synthetic \n,
// but LT suggests a space instead (https://github.com/languagetool-org/languagetool/issues/5297)
private fun commonPrefixLength(s1: CharSequence, s2: CharSequence): Int {
val minLength = min(s1.length, s2.length)
var i = 0
while (i < minLength && charsMatch(s1[i], s2[i])) i++
return i
}
private fun commonSuffixLength(s1: CharSequence, s2: CharSequence): Int {
val minLength = min(s1.length, s2.length)
var i = 0
while (i < minLength && charsMatch(s1[s1.length - i - 1], s2[s2.length - i - 1])) i++
return i
}
private fun charsMatch(c1: Char, c2: Char) = c1 == c2 || c1 == ' ' && c2 == '\n'
}

View File

@@ -18,7 +18,7 @@ internal class StrategyTextExtractor(private val strategy: GrammarCheckingStrate
.filter { it !is PsiCompiledElement && !wsTokens.contains(it.elementType) }
.mapNotNull { buildTextContent(it) }
.mapNotNull { trimLeadingQuotesAndSpaces(it) }
return TextContent.joinWithWhitespace(fragments)
return TextContent.joinWithWhitespace('\n', fragments)
}
private fun trimLeadingQuotesAndSpaces(content: TextContent): TextContent? {

View File

@@ -2,6 +2,7 @@ package com.intellij.grazie.text;
import com.intellij.openapi.util.TextRange;
import com.intellij.openapi.util.UserDataHolderEx;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiFile;
import com.intellij.util.containers.ContainerUtil;
@@ -190,18 +191,32 @@ public interface TextContent extends CharSequence, UserDataHolderEx {
/**
* @return a concatenation of several text contents (which must have the same domains)
* with a single synthetic space character inserted between each pair of adjacent components.
* with a single synthetic ' ' character inserted between each pair of adjacent components.
* @deprecated use {@link #joinWithWhitespace(char, List)}
*/
@Nullable
@Deprecated
static TextContent joinWithWhitespace(List<? extends @NotNull TextContent> components) {
return joinWithWhitespace(' ', components);
}
/**
* @return a concatenation of several text contents (which must have the same domains)
* with the given whitespace character inserted between each pair of adjacent components.
*/
@Nullable
static TextContent joinWithWhitespace(char whitespace, List<? extends @NotNull TextContent> components) {
if (!Character.isWhitespace(whitespace)) {
throw new IllegalArgumentException("Whitespace expected, got " + StringUtil.escapeStringCharacters(String.valueOf(whitespace)));
}
if (components.isEmpty()) return null;
if (components.size() == 1) return components.get(0);
return new TextContentImpl(commonDomain(components),
StreamEx.of(components)
.map(c -> ((TextContentImpl) c).tokens)
.intersperse(Collections.singletonList(TextContentImpl.WS_TOKEN))
.toFlatList(Function.identity()));
TextContentImpl.WSTokenInfo wsToken = new TextContentImpl.WSTokenInfo(whitespace);
return new TextContentImpl(commonDomain(components), StreamEx.of(components)
.map(c -> ((TextContentImpl) c).tokens)
.intersperse(Collections.singletonList(wsToken))
.toFlatList(Function.identity()));
}
private static TextDomain commonDomain(List<? extends TextContent> components) {

View File

@@ -39,8 +39,8 @@ class TextContentImpl extends UserDataHolderBase implements TextContent {
}
tokens.add(token);
}
if (tokens.get(0) == WS_TOKEN) tokens.remove(0);
if (tokens.get(tokens.size() - 1) == WS_TOKEN) tokens.remove(tokens.size() - 1);
if (tokens.get(0) instanceof WSTokenInfo) tokens.remove(0);
if (tokens.get(tokens.size() - 1) instanceof WSTokenInfo) tokens.remove(tokens.size() - 1);
if (tokens.isEmpty()) {
throw new IllegalArgumentException("There should be at least one non-whitespace token");
}
@@ -60,11 +60,11 @@ class TextContentImpl extends UserDataHolderBase implements TextContent {
if (leanForward) {
while (index < tokens.size() - 1 && tokens.get(index).length() == 0) index++;
if (tokens.get(index) == WS_TOKEN) {
if (tokens.get(index) instanceof WSTokenInfo) {
index--;
}
} else {
if (index > 0 && tokens.get(index - 1) != WS_TOKEN) index--;
if (index > 0 && !(tokens.get(index - 1) instanceof WSTokenInfo)) index--;
}
return index;
@@ -323,7 +323,7 @@ class TextContentImpl extends UserDataHolderBase implements TextContent {
}
private static @Nullable TokenInfo merge(TokenInfo t1, TokenInfo t2) {
if (t1 == WS_TOKEN && t2 == WS_TOKEN) return t1;
if (t1 instanceof WSTokenInfo && t2 instanceof WSTokenInfo) return t1;
if (t1 instanceof PsiToken && t2 instanceof PsiToken) {
if (((PsiToken) t1).unknown && ((PsiToken) t2).unknown) {
return t1;
@@ -336,8 +336,6 @@ class TextContentImpl extends UserDataHolderBase implements TextContent {
return null;
}
static final TokenInfo WS_TOKEN = new TokenInfo(" ") {};
abstract static class TokenInfo {
final String text;
@@ -422,4 +420,8 @@ class TextContentImpl extends UserDataHolderBase implements TextContent {
return shreds;
}
}
static class WSTokenInfo extends TokenInfo {
WSTokenInfo(char ws) { super(String.valueOf(ws)); }
}
}

View File

@@ -81,7 +81,7 @@ public class TextContentTest extends BasePlatformTestCase {
var f1 = psiFragment(file, 0, 1);
var f2 = psiFragment(file, 4, 5).markUnknown(new TextRange(1, 1));
var joined = TextContent.joinWithWhitespace(List.of(f1, f2));
var joined = TextContent.joinWithWhitespace(' ', List.of(f1, f2));
assertNotNull(joined);
assertEquals("a c|", unknownOffsets(joined));
assertEquals(List.of(0, 1, 4, 5), IntStreamEx.range(4).mapToObj(joined::textOffsetToFile).toList());

View File

@@ -44,15 +44,17 @@ public class TextExtractionTest extends BasePlatformTestCase {
String text = extractText("a.java", "//Hello. I are a very humble\n//persons.\n\nclass C {}", 4).toString();
assertTrue(text, text.matches("Hello\\. I are a very humble\\spersons\\."));
assertEquals("First line. Third line.", extractText("a.java",
assertEquals("First line.\nThird line.", extractText("a.java",
"// First line.\n" +
"// \n" +
"// Third line.\n"
, 4).toString());
text = "//1\n//2\n//3\n//4";
assertEquals("1 2 3 4", extractText("a.java", text, text.indexOf("1")).toString());
assertEquals("1 2 3 4", extractText("a.java", text, text.indexOf("3")).toString());
PsiFile file = PsiFileFactory.getInstance(getProject()).createFileFromText("a.java", JavaFileType.INSTANCE, text);
TextContent tc = TextExtractor.findTextAt(file, text.indexOf("1"), TextContent.TextDomain.ALL);
assertEquals("1\n2\n3\n4", tc.toString());
assertEquals(tc, TextExtractor.findTextAt(file, text.indexOf("3"), TextContent.TextDomain.ALL));
}
public void testIgnorePropertyCommentStarts() {
@@ -73,7 +75,7 @@ public class TextExtractionTest extends BasePlatformTestCase {
}
public void testMultiLineCommentInProperties() {
assertEquals("line1 line2", TextContentTest.unknownOffsets(extractText("a.properties", "# line1\n! line2", 4)));
assertEquals("line1\nline2", TextContentTest.unknownOffsets(extractText("a.properties", "# line1\n! line2", 4)));
}
public void testJavadoc() {

View File

@@ -5,6 +5,9 @@ class ForMultiLanguageSupport {
// das ist <warning descr="FUEHR_FUER">führ</warning> Dich!
// das <TYPO descr="Typo: In word 'daert'">daert</TYPO> geschätzt fünf <warning descr="MANNSTUNDE">Mannstunden</warning>.
// Cover following cases
// a) initially missing
// b) initially missing
// My
// <warning descr="COMMA_WHICH">name</warning>

View File

@@ -17,7 +17,7 @@ class YamlTextExtractor : TextExtractor() {
when (root.node.elementType) {
COMMENT -> {
val siblings = getNotSoDistantSimilarSiblings(root, TokenSet.create(WHITESPACE, INDENT, EOL)) { it.elementType == COMMENT }
return TextContent.joinWithWhitespace(siblings.mapNotNull { commentBuilder.build(it, TextContent.TextDomain.COMMENTS) })
return TextContent.joinWithWhitespace('\n', siblings.mapNotNull { commentBuilder.build(it, TextContent.TextDomain.COMMENTS) })
}
TEXT, SCALAR_STRING, SCALAR_DSTRING, SCALAR_LIST, SCALAR_TEXT ->
return TextContentBuilder.FromPsi.excluding { isStealth(it) }.build(root, TextContent.TextDomain.LITERALS)

View File

@@ -37,7 +37,7 @@ internal class KotlinTextExtractor : TextExtractor() {
val roots = getNotSoDistantSimilarSiblings(root) {
it == root || root.elementType == KtTokens.EOL_COMMENT && it.elementType == KtTokens.EOL_COMMENT
}
return TextContent.joinWithWhitespace(roots.mapNotNull {
return TextContent.joinWithWhitespace('\n', roots.mapNotNull {
TextContentBuilder.FromPsi.removingIndents(" \t*/").build(it, COMMENTS)
})
}

View File

@@ -23,7 +23,7 @@ internal class PythonTextExtractor : TextExtractor() {
if (root is PsiCommentImpl) {
val siblings = getNotSoDistantSimilarSiblings(root) { it is PsiCommentImpl }
return TextContent.joinWithWhitespace(siblings.mapNotNull { TextContent.builder().build(it, TextContent.TextDomain.COMMENTS) })
return TextContent.joinWithWhitespace('\n', siblings.mapNotNull { TextContent.builder().build(it, TextContent.TextDomain.COMMENTS) })
}
return null