Skip to content

fix: maintain area for replacing service #988

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

fix: maintain area for replacing service #988

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions server/src/main/java/org/eclipse/lsp/cobol/core/preprocessor/ProcessingConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,6 @@ public final class ProcessingConstants {
public static final int START_INDEX_AREA_A = 4;
public static final int END_INDEX_CONTENT_AREA_A = 10;
public static final int INDICATOR_AREA = 7;
public static final int END_INDEX_AREA_B = 72;
public static final String CONTINUATION_PREFIX = " - ";
}
84 changes: 83 additions & 1 deletion ...ain/java/org/eclipse/lsp/cobol/core/preprocessor/delegates/util/ReplacingServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,18 @@

package org.eclipse.lsp.cobol.core.preprocessor.delegates.util;

import com.google.common.base.Splitter;
import com.google.inject.Singleton;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NonNull;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.tuple.Pair;
import org.eclipse.lsp.cobol.core.model.ErrorSeverity;
import org.eclipse.lsp.cobol.core.model.ResultWithErrors;
import org.eclipse.lsp.cobol.core.model.SyntaxError;
import org.eclipse.lsp.cobol.core.preprocessor.ProcessingConstants;

import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -35,6 +38,7 @@

import static java.lang.String.format;
import static java.util.regex.Matcher.quoteReplacement;
import static org.eclipse.lsp.cobol.core.preprocessor.ProcessingConstants.END_INDEX_AREA_B;

/**
* This service applies replacing for given text by replace clauses and tokens. It may work with
Expand Down Expand Up @@ -209,12 +213,90 @@ private String replace(@NonNull String text, @NonNull Pair<String, String> patte
String result = text;
try {
result = Pattern.compile(pattern.getLeft()).matcher(text).replaceAll(pattern.getRight());
List<String> fixedStrings = validateAndFix(result, text);
result = String.join(System.lineSeparator(), fixedStrings);
} catch (IndexOutOfBoundsException e) {
LOG.error(format(ERROR_REPLACING, text, pattern.toString()), e);
LOG.error(format(ERROR_REPLACING, text, pattern), e);
}
return result;
}

private List<String> validateAndFix(String result, String referenceText) {
List<String> referenceStringList =
new ArrayList<>(Arrays.asList(referenceText.split(System.lineSeparator())));
List<String> resultStringList =
new ArrayList<>(Arrays.asList(result.split(System.lineSeparator())));
int resultIndex = 0;
for (int refIndex = 0; refIndex < referenceStringList.size(); refIndex++) {
int lengthDiff =
resultStringList.get(resultIndex).length() - referenceStringList.get(refIndex).length();

if (lengthDiff == 0) {
resultIndex++;
continue;
}
if (resultStringList.get(resultIndex).length() > END_INDEX_AREA_B) {

String sequence =
referenceStringList.get(refIndex).length() > END_INDEX_AREA_B
? referenceStringList.get(refIndex).substring(END_INDEX_AREA_B)
: "";
if (lengthDiff < 0 && StringUtils.isNumeric(sequence)) {
resultStringList.set(
resultIndex, adjustSequenceNoPresentInAreaB(resultStringList, resultIndex, sequence));
} else {
int prevSize = resultStringList.size();
adjustExtraLength(resultStringList, refIndex, resultIndex, sequence);
resultIndex = resultIndex + resultStringList.size() - prevSize;
}
}
resultIndex++;
}
return resultStringList;
}

private String adjustSequenceNoPresentInAreaB(
List<String> resultStringList, int j, String sequence) {
return StringUtils.join(
StringUtils.rightPad(
StringUtils.substring(
resultStringList.get(j), 0, resultStringList.get(j).length() - sequence.length()),
END_INDEX_AREA_B),
sequence);
}

private void adjustExtraLength(
List<String> resultStringList, int refIndex, int resultIndex, String sequence) {
String addedString =
resultStringList
.get(resultIndex)
.substring(
END_INDEX_AREA_B, resultStringList.get(resultIndex).length() - sequence.length());
resultStringList.set(
refIndex,
StringUtils.join(
StringUtils.substring(resultStringList.get(resultIndex), 0, END_INDEX_AREA_B),
sequence));
if (!StringUtils.isBlank(addedString)) {
List<String> adjustedExtraCharInContinuationLine =
splitStringByLength(
addedString, END_INDEX_AREA_B - ProcessingConstants.CONTINUATION_PREFIX.length());
resultStringList.addAll(resultIndex + 1, adjustedExtraCharInContinuationLine);
}
}

private List<String> splitStringByLength(String actualStr, int fixedLength) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you change a number of strings and I'm afraid that this will change our mapping. I tried to reproduce the case when we need it and found this bug.
COBOL program:

0      IDENTIFICATION DIVISION.
1      PROGRAM-ID. TESTREPL.
2      DATA DIVISION.
3      WORKING-STORAGE SECTION.
       01  LOGA.  
       COPY REPL5  REPLACING ==LDAY== BY ==DMAN12345789sadfasfdsafasdfsa
      -    asdfsafsfabfgbtdndfrg==.

       PROCEDURE DIVISION.
           MOVE 0 TO DMAN12345789sadfasfdsafasdfsaasdfsafsfabfgbtdndfrg.

COPYBOOK REPL5.

      ***************************************************************** 09700000
         02  LOGHDR.                                                    18000000
            03  LDAY           PIC S9(7) COMP-3.

I see that after the preprocessor changed line with 03 LDAY is very long and not split. This also causes some errors to the user. Did you try to solve issues like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I tried something like this. I actually hoped that it won't give any mapping issues as the replacement with continued lines happens before we start the actual pre-processing. And the long replacement text is should be because the pre-processor concatenates. But I guess, I missed some scenarios. I will look into it. Thanks :)
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. That's interesting because you are showing the exact case I did, but you don't have any errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was my mistake. I have added a use-case test for this scenario.

List<String> splitReplacement = new ArrayList<>();
Splitter.fixedLength(fixedLength)
.split(actualStr)
.forEach(
ele -> {
if (!StringUtils.isBlank(ele))
splitReplacement.add(ProcessingConstants.CONTINUATION_PREFIX + ele);
});
return splitReplacement;
}

private Function<String, Boolean> checkContainWord(String check) {
return text ->
Arrays.stream(text.toUpperCase().split("\b")).anyMatch(txt -> txt.equalsIgnoreCase(check));
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/org/eclipse/lsp/cobol/service/utils/WorkspaceFileService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public String getContentByPath(@NonNull Path path) {
StringBuilder sb = new StringBuilder();
String line = bufferedReader.readLine();
while (line != null) {
sb.append(line).append("\n");
sb.append(line).append(System.lineSeparator());
line = bufferedReader.readLine();
}
return sb.toString();
Expand Down Expand Up @@ -111,7 +111,7 @@ public String readFromInputStream(InputStream inputStream, Charset charset) thro
try (BufferedReader br = new BufferedReader(new InputStreamReader(inputStream, charset))) {
String line;
while ((line = br.readLine()) != null) {
resultStringBuilder.append(line).append("\n");
resultStringBuilder.append(line).append(System.lineSeparator());
}
}
return resultStringBuilder.toString();
Expand Down
65 changes: 60 additions & 5 deletions server/src/test/java/org/eclipse/lsp/cobol/usecases/TestCopybookReplacePatterns.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,21 +56,58 @@ class TestCopybookReplacePatterns {
private static final String TEXT4 =
BASE
+ "5 COPY {~REPL4} REPLACING TRAILING ==ID== BY == BY-IDS ==.\n"
+ "8 PROCEDURE DIVISION.\n"
+ "8 PROCEDURE DIVISION.\r\n"
+ "9 MOVE 0 TO {$TAG_BY-IDS}.";
private static final String TEXT5 =
BASE
+ " 01 {$*LOGA}. \r\n"
+ " COPY {~REPL5} \r\n"
+ " REPLACING ==LDAY== BY ==DMAN123000000000000000000000000000000005900\n"
+ " - 0000000000000000011111111111111111111111111111111111111111111\n"
+ " - 00000000000000000000000== .\r\n";

private static final String TEXT6 =
BASE
+ "5 COPY {~REPL6} REPLACING ==:TAG:== BY == A ==.\r\n"
+ "8 PROCEDURE DIVISION.\r\n"
+ "9 MOVE 0 TO {$A_ID}.";

private static final String TEXT7 =
BASE
+ " 01 {$*LOGA}. \r\n"
+ " COPY {~REPL7} \r\n"
+ " REPLACING ==LDAY== BY ==DMAN123000000000000000000000000000000005900\n"
+ " - 0000000000000000011111111111111111111111111111111111111111111\n"
+ " - 00000000000000000000000== .\r\n";

private static final String REPL = "0 01 {$*TAG_ID} PIC 9.\n";
private static final String REPL = "0 01 {$*TAG_ID} PIC 9.\r\n";
private static final String REPL_NAME = "REPL";

private static final String REPL1 = "0 01 {$*:TAG:_ID^ACC_ID} PIC 9.\n";
private static final String REPL1 = "0 01 {$*:TAG:_ID^ACC_ID} PIC 9.\r\n";
private static final String REPL1_NAME = "REPL1";

private static final String REPL3 = "0 01 {$*TAG_ID^ACC_ID} PIC 9.\n";
private static final String REPL3 = "0 01 {$*TAG_ID^ACC_ID} PIC 9.\r\n";
private static final String REPL3_NAME = "REPL3";

private static final String REPL4 = "0 01 {$*TAG_ID^TAG_BY-IDS} PIC 9.\n";
private static final String REPL4 = "0 01 {$*TAG_ID^TAG_BY-IDS} PIC 9.\r\n";
private static final String REPL4_NAME = "REPL4";

private static final String REPL5 =
" ***************************************************************** 09700000\r\n"
+ " 02 {$*LOGHDR}. 18000000\r\n"
+ " 03 {$*LDAY^DMAN1230000000000000000000000000000000000000000000001111111111111111111111111111111111111111111100000000000000000000000} PIC S9(7) COMP-3. 24000000";
private static final String REPL5_NAME = "REPL5";

private static final String REPL6 =
"0 01 {$*:TAG:_ID^A_ID} PIC 9. 00007100";
private static final String REPL6_NAME = "REPL6";

private static final String REPL7 =
" ***************************************************************** 09700000\r\n"
+ " 02 {$*LOGHDR}. 18000000\r\n"
+ " 03 {$*LDAY^DMAN1230000000000000000000000000000000000000000000001111111111111111111111111111111111111111111100000000000000000000000} PIC S9(7) COMP-3.";
private static final String REPL7_NAME = "REPL7";

@Test
void testPartialTextAreNotReplaced() {
UseCaseEngine.runTest(
Expand Down Expand Up @@ -102,4 +139,22 @@ void testPartialTextReplaceableWithTrailingClause() {
UseCaseEngine.runTest(
TEXT4, ImmutableList.of(new CobolText(REPL4_NAME, REPL4)), ImmutableMap.of());
}

@Test
void testWhenReplacedLengthIsMoreThanReplaceable() {
UseCaseEngine.runTest(
TEXT5, ImmutableList.of(new CobolText(REPL5_NAME, REPL5)), ImmutableMap.of());
}

@Test
void testWhenReplacedLengthIsLessThanReplaceable() {
UseCaseEngine.runTest(
TEXT6, ImmutableList.of(new CobolText(REPL6_NAME, REPL6)), ImmutableMap.of());
}

@Test
void testWhenReplacedLengthIsMoreThanReplaceableAndCopybookHasNoSequence() {
UseCaseEngine.runTest(
TEXT7, ImmutableList.of(new CobolText(REPL7_NAME, REPL7)), ImmutableMap.of());
}
}
35 changes: 18 additions & 17 deletions ...est/java/org/eclipse/lsp/cobol/usecases/TestReplacingIdentifiersAppliedForFullTokens.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@

package org.eclipse.lsp.cobol.usecases;

import org.eclipse.lsp.cobol.positive.CobolText;
import org.eclipse.lsp.cobol.usecases.engine.UseCaseEngine;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import org.eclipse.lsp.cobol.positive.CobolText;
import org.eclipse.lsp.cobol.usecases.engine.UseCaseEngine;
import org.junit.jupiter.api.Test;

/**
Expand All @@ -27,30 +27,31 @@
*/
class TestReplacingIdentifiersAppliedForFullTokens {
private static final String TEXT =
"0 IDENTIFICATION DIVISION.\n"
+ "1 PROGRAM-ID. TESTREPL.\n"
+ "2 DATA DIVISION.\n"
+ "3 WORKING-STORAGE SECTION.\n"
+ "4 COPY {~REPL} REPLACING A BY VARB\n"
+ "5 B BY VARC\n"
+ "6 C BY VARD\n"
+ "7 D BY VARE.\n"
+ "8 PROCEDURE DIVISION.\n"
+ "9 {#*MAINLINE}.\n"
+ "10 MOVE 00 TO {$VARC} OF {$VARB}.\n"
"0 IDENTIFICATION DIVISION.\r\n"
+ "1 PROGRAM-ID. TESTREPL.\r\n"
+ "2 DATA DIVISION.\r\n"
+ "3 WORKING-STORAGE SECTION.\r\n"
+ "4 COPY {~REPL} REPLACING A BY VARB\r\n"
+ "5 B BY VARC\r\n"
+ "6 C BY VARD\r\n"
+ "7 D BY VARE.\r\n"
+ "8 PROCEDURE DIVISION.\r\n"
+ "9 {#*MAINLINE}.\r\n"
+ "10 MOVE 00 TO {$VARC} OF {$VARB}.\r\n"
+ "11 GOBACK. ";

private static final String REPL =
"0 01 {$*A^VARB}.\n"
+ "1 02 {$*B^VARC} PIC S99.\n"
+ "2 02 {$*C^VARD} PIC S9(5)V99.\n"
+ "3 02 {$*D^VARE} PIC S99 OCCURS 1 TO 10 TIMES\n"
+ "1 02 {$*B^VARC} PIC S99.\r\n"
+ "2 02 {$*C^VARD} PIC S9(5)V99.\r\n"
+ "3 02 {$*D^VARE} PIC S99 OCCURS 1 TO 10 TIMES\r\n"
+ "4 DEPENDING ON {$B^VARC} OF {$A^VARB}.";

private static final String REPL_NAME = "REPL";

@Test
void test() {
UseCaseEngine.runTest(TEXT, ImmutableList.of(new CobolText(REPL_NAME, REPL)), ImmutableMap.of());
UseCaseEngine.runTest(
TEXT, ImmutableList.of(new CobolText(REPL_NAME, REPL)), ImmutableMap.of());
}
}