Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: Fix snippet text for sections and paragraphs #2533

Merged
merged 2 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.util.List;

import org.eclipse.lsp.cobol.common.model.Locality;
import org.eclipse.lsp4j.Location;
import org.eclipse.lsp4j.Position;
import org.eclipse.lsp4j.Range;
Expand Down Expand Up @@ -54,6 +55,29 @@ public String getUri() {
return baseText.getUri();
}

/**
* Returns original text, situated between the start and the end lines from the locality range
* @param locality - the text locality
* @return string value that represents a text from the original document
*/
public String getBaseText(Locality locality) {
Nurkambay marked this conversation as resolved.
Show resolved Hide resolved
int startLine = locality.getRange().getStart().getLine();
int endLine = locality.getRange().getEnd().getLine();;

if (!baseText.getUri().equals(locality.getUri())
|| baseText.getLines().size() <= endLine) {
return "";
}
StringBuilder sb = new StringBuilder();
for (int i = startLine; i <= endLine; i++) {
sb.append(baseText.getLines().get(i).toString());
if (i != endLine) {
sb.append("\r\n");
}
}
return sb.toString();
}

/**
* Commit changes
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/
package org.eclipse.lsp.cobol.common.mapping;

import org.eclipse.lsp.cobol.common.model.Locality;
import org.eclipse.lsp4j.Location;
import org.eclipse.lsp4j.Position;
import org.eclipse.lsp4j.Range;
Expand Down Expand Up @@ -122,4 +123,44 @@ void testIsLineEmptyBetweenColumns() {
assertFalse(document.isLineEmptyBetweenColumns(0, 7, 100));
assertTrue(document.isLineEmptyBetweenColumns(1000, 0, 1));
}

@Test
void testGetBaseTextProperUri() {
document.insertCopybook(5, copybook);
Locality locality = Locality.builder()
.uri(documentUri)
.range(new Range(new Position(3, 5), new Position(4, 8)))
.build();
String text = document.getBaseText(locality);

assertEquals(" WORKING-STORAGE SECTION.\r\n" +
" COPY CPY.", text);
}

@Test
void testGetBaseTextEdge() {
document.insertCopybook(5, copybook);
Locality locality = Locality.builder()
.uri(documentUri)
.range(new Range(new Position(3, 5), new Position(5, 1)))
.build();
String text = document.getBaseText(locality);

assertEquals(" WORKING-STORAGE SECTION.\r\n" +
" COPY CPY.\r\n" +
" PROCEDURE DIVISION.", text);
}

@Test
void testGetBaseTextWrongUri() {
document.insertCopybook(5, copybook);
Locality locality = Locality.builder()
.uri("wrong")
.range(new Range(new Position(3, 5), new Position(4, 8)))
.build();
String text = document.getBaseText(locality);

assertEquals("", text);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import com.google.common.collect.ImmutableList;
import lombok.NonNull;
import org.antlr.v4.runtime.*;
import org.antlr.v4.runtime.misc.Interval;
import org.antlr.v4.runtime.tree.ParseTree;
import org.antlr.v4.runtime.tree.RuleNode;
import org.antlr.v4.runtime.tree.TerminalNode;
Expand Down Expand Up @@ -110,7 +109,6 @@ public CobolVisitor(
@Override
public List<Node> visitStartRule(StartRuleContext ctx) {
// we can skip the other nodes, but not the root
text.setInputStream(ctx.getStart().getInputStream());
try {
return ImmutableList.of(
retrieveLocality(ctx, extendedDocument, copybooks)
Expand Down Expand Up @@ -1856,7 +1854,6 @@ public List<SyntaxError> getErrors() {

private static final class TextExtractionState {
private final ExtendedDocument extendedDocument;
private CharStream input = null;
private ParagraphNode lastParagraphNode = null;
private ProcedureSectionNode lastSectionNode = null;
private Token firstSectionToken = null;
Expand All @@ -1870,26 +1867,23 @@ private TextExtractionState(ExtendedDocument extendedDocument) {

void update(ProcedureSectionNode section, Token firstToken) {
if (lastParagraphNode != null) {
String text = input.getText(new Interval(firstParagraphToken.getStartIndex(), lastSentenseToken.getStopIndex()));
lastParagraphNode.setText(text);
lastParagraphNode.getLocality().getRange().setEnd(lastSentensePosition);
lastParagraphNode.setText(extendedDocument.getBaseText(lastParagraphNode.getLocality()));
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be possible to delay this action to the point where it is actually requrested?
And perhaps consider the needs of the only user of this data CFASTBuilderImpl.cutSnippet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, but in that case we need to pass document text to ProcessingResult to satisfy CliCFAST.
Maybe it is okay

lastParagraphNode = null;
firstParagraphToken = null;
}
if (lastSectionNode != null) {
String text = input.getText(new Interval(firstSectionToken.getStartIndex(), lastSentenseToken.getStopIndex()));
lastSectionNode.getLocality().getRange().setEnd(lastSentensePosition);
lastSectionNode.setText(text);
lastSectionNode.setText(extendedDocument.getBaseText(lastSectionNode.getLocality()));
}
firstSectionToken = firstToken;
lastSectionNode = section;
}

void update(ParagraphNode paragraphNode, Token firstToken) {
if (lastParagraphNode != null) {
String text = input.getText(new Interval(firstParagraphToken.getStartIndex(), lastSentenseToken.getStopIndex()));
lastParagraphNode.setText(text);
lastParagraphNode.getLocality().getRange().setEnd(lastSentensePosition);
lastParagraphNode.setText(extendedDocument.getBaseText(lastParagraphNode.getLocality()));
}
lastParagraphNode = paragraphNode;
firstParagraphToken = firstToken;
Expand All @@ -1901,10 +1895,6 @@ void update(Token lastToken) {
.getRange().getEnd();
}

void setInputStream(CharStream input) {
this.input = input;
}

void reset() {
lastParagraphNode = null;
lastSectionNode = null;
Expand All @@ -1915,14 +1905,12 @@ void reset() {
}
void flush() {
if (lastParagraphNode != null) {
String text = input.getText(new Interval(firstParagraphToken.getStartIndex(), lastSentenseToken.getStopIndex()));
lastParagraphNode.setText(text);
lastParagraphNode.getLocality().getRange().setEnd(lastSentensePosition);
lastParagraphNode.setText(extendedDocument.getBaseText(lastParagraphNode.getLocality()));
}
if (lastSectionNode != null) {
String text = input.getText(new Interval(firstSectionToken.getStartIndex(), lastSentenseToken.getStopIndex()));
lastSectionNode.setText(text);
lastSectionNode.getLocality().getRange().setEnd(lastSentensePosition);
lastSectionNode.setText(extendedDocument.getBaseText(lastSectionNode.getLocality()));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,16 @@ public ExtendedApiResult analysis(AnalysisResultEvent analysisResultEvent) throw
int character = analysisResultEvent.getCharacter();
Position position = new Position(line, character);
Optional<Node> selectedNode = RangeUtils.findNodeByPosition(rootNode, analysisResultEvent.getUri(), position);

ProgramNode programNode;
if (selectedNode.isPresent() && !(selectedNode.get() instanceof RootNode)) {
return cfastBuilder.build((ProgramNode) selectedNode.get()
.getNearestParentByType(PROGRAM).orElse(selectProgramNode(rootNode.findPrograms(), line, character)));
programNode = (ProgramNode) selectedNode.get()
.getNearestParentByType(PROGRAM).orElse(selectProgramNode(rootNode.findPrograms(), line, character));
} else {
programNode = selectProgramNode(rootNode.findPrograms(), line, character);
}

return cfastBuilder.build(selectProgramNode(rootNode.findPrograms(), line, character));
return cfastBuilder.build(programNode);
}

private ProgramNode selectProgramNode(List<ProgramNode> programs, int line, int character) {
Expand Down
4 changes: 2 additions & 2 deletions server/engine/src/test/resources/cfast/case3.result.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
}
},
{
"snippet": "PARAG1.\r\n DISPLAY \u0027PARAG1\u0027.",
"snippet": " PARAG1.\r\n DISPLAY \u0027PARAG1\u0027.",
"name": "PARAG1",
"type": "paragraph",
"location": {
Expand Down Expand Up @@ -61,7 +61,7 @@
]
},
{
"snippet": "PARAG2.\r\n DISPLAY \u0027PARAG2\u0027.",
"snippet": " PARAG2.\r\n DISPLAY \u0027PARAG2\u0027.",
"name": "PARAG2",
"type": "paragraph",
"location": {
Expand Down
4 changes: 2 additions & 2 deletions server/engine/src/test/resources/cfast/case4.result.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
}
},
{
"snippet": "PARAG1.\r\n DISPLAY \u0027PARAG1\u0027.\r\n PERFORM PARAG2.",
"snippet": " PARAG1.\r\n DISPLAY \u0027PARAG1\u0027.\r\n PERFORM PARAG2.",
"name": "PARAG1",
"type": "paragraph",
"location": {
Expand Down Expand Up @@ -93,7 +93,7 @@
]
},
{
"snippet": "PARAG2.\r\n DISPLAY \u0027PARAG2\u0027.",
"snippet": " PARAG2.\r\n DISPLAY \u0027PARAG2\u0027.",
"name": "PARAG2",
"type": "paragraph",
"location": {
Expand Down
6 changes: 3 additions & 3 deletions server/engine/src/test/resources/cfast/case46.result.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
},
"children": [
{
"snippet": "P8000-READ-TABLE.\r\n \r\n READ TRAVCONV-FILE INTO TRAVELERS-CONV-RECORD\r\n AT END GO TO P8000-EXIT.\r\n MOVE \u0027Y\u0027 TO OUT-REC-TYPE.\r\n PERFORM X1.",
"snippet": " P8000-READ-TABLE.\r\n \r\n READ TRAVCONV-FILE INTO TRAVELERS-CONV-RECORD\r\n AT END GO TO P8000-EXIT.\r\n MOVE \u0027Y\u0027 TO OUT-REC-TYPE.\r\n PERFORM X1.",
"name": "P8000-READ-TABLE",
"type": "paragraph",
"location": {
Expand Down Expand Up @@ -121,7 +121,7 @@
]
},
{
"snippet": "X1.\r\n DISPLAY \u0027HAHAH\u0027.",
"snippet": " X1.\r\n DISPLAY \u0027HAHAH\u0027.",
"name": "X1",
"type": "paragraph",
"location": {
Expand Down Expand Up @@ -153,7 +153,7 @@
]
},
{
"snippet": "P8000-EXIT.\r\n EXIT.",
"snippet": " P8000-EXIT.\r\n EXIT.",
"name": "P8000-EXIT",
"type": "paragraph",
"location": {
Expand Down
8 changes: 4 additions & 4 deletions server/engine/src/test/resources/cfast/case5.result.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
},
{
"name": "SECT1",
"snippet": "SECT1 SECTION.\r\n PARAG1.\r\n DISPLAY \u0027PARAG1\u0027.\r\n PERFORM PARAG2 OF SECT2.",
"snippet": " SECT1 SECTION.\r\n PARAG1.\r\n DISPLAY \u0027PARAG1\u0027.\r\n PERFORM PARAG2 OF SECT2.",
"type": "section",
"location": {
"uri": "fake/path",
Expand All @@ -62,7 +62,7 @@
},
"children": [
{
"snippet": "PARAG1.\r\n DISPLAY \u0027PARAG1\u0027.\r\n PERFORM PARAG2 OF SECT2.",
"snippet": " PARAG1.\r\n DISPLAY \u0027PARAG1\u0027.\r\n PERFORM PARAG2 OF SECT2.",
"name": "PARAG1",
"type": "paragraph",
"location": {
Expand Down Expand Up @@ -113,7 +113,7 @@
},
{
"name": "SECT2",
"snippet": "SECT2 SECTION.\r\n PARAG2.\r\n DISPLAY \u0027PARAG2\u0027.",
"snippet": " SECT2 SECTION.\r\n PARAG2.\r\n DISPLAY \u0027PARAG2\u0027.",
"type": "section",
"location": {
"uri": "fake/path",
Expand All @@ -128,7 +128,7 @@
},
"children": [
{
"snippet": "PARAG2.\r\n DISPLAY \u0027PARAG2\u0027.",
"snippet": " PARAG2.\r\n DISPLAY \u0027PARAG2\u0027.",
"name": "PARAG2",
"type": "paragraph",
"location": {
Expand Down
2 changes: 1 addition & 1 deletion server/engine/src/test/resources/cfast/case7.result.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
}
},
{
"snippet": "PARAG1.\r\n EXIT.",
"snippet": " PARAG1.\r\n EXIT.",
"name": "PARAG1",
"type": "paragraph",
"location": {
Expand Down
2 changes: 1 addition & 1 deletion server/engine/src/test/resources/cfast/case8.result.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
}
},
{
"snippet": "PARAG1.\r\n EXIT.",
"snippet": " PARAG1.\r\n EXIT.",
"name": "PARAG1",
"type": "paragraph",
"location": {
Expand Down
6 changes: 3 additions & 3 deletions server/engine/src/test/resources/cfast/case_alter.result.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
},
{
"name": "SEC1",
"snippet": "SEC1 SECTION.\r\n PAR1.\r\n PAR2.",
"snippet": " SEC1 SECTION.\r\n PAR1.\r\n PAR2.",
"type": "section",
"location": {
"uri": "fake/path",
Expand All @@ -52,7 +52,7 @@
},
"children": [
{
"snippet": "PAR1.",
"snippet": " PAR1.",
"name": "PAR1",
"type": "paragraph",
"location": {
Expand All @@ -68,7 +68,7 @@
}
},
{
"snippet": "PAR2.",
"snippet": " PAR2.",
"name": "PAR2",
"type": "paragraph",
"location": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
}
},
{
"snippet": "PAR1.\r\n CONTINUE.",
"snippet": " PAR1.\r\n CONTINUE.",
"name": "PAR1",
"type": "paragraph",
"location": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
}
},
{
"snippet": "PAR1.\r\n CONTINUE.",
"snippet": " PAR1.\r\n CONTINUE.",
"name": "PAR1",
"type": "paragraph",
"location": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@
}
},
{
"snippet": "PAR1.\r\n CONTINUE.",
"snippet": " PAR1.\r\n CONTINUE.",
"name": "PAR1",
"type": "paragraph",
"location": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
}
},
{
"snippet": "HANDLE-ABEND.",
"snippet": " HANDLE-ABEND.",
"name": "HANDLE-ABEND",
"type": "paragraph",
"location": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@
}
},
{
"snippet": "HANDLER.\r\n DISPLAY \"HANDLER\".",
"snippet": " HANDLER.\r\n DISPLAY \"HANDLER\".",
"name": "HANDLER",
"type": "paragraph",
"location": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
}
},
{
"snippet": "OPTIONS.\r\n DISPLAY \u0027OPTION\u0027.",
"snippet": " OPTIONS.\r\n DISPLAY \u0027OPTION\u0027.",
"name": "OPTIONS",
"type": "paragraph",
"location": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
},
{
"name": "SEC1",
"snippet": "SEC1 SECTION.",
"snippet": " SEC1 SECTION.",
"type": "section",
"location": {
"uri": "fake/path",
Expand Down
Loading
Loading