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

#4078 [runtime/java] fixed npe due to CommonToken.getText() returning null when inputStream is null #4251

Closed

Conversation

vityaman
Copy link

@vityaman vityaman commented Apr 30, 2023

Solution of Issue 4078: #4078

The problem was that CommonToken::getText() returns null if and only if it has InputStream field null, what happens when we call CommonToken(int type) as there source is equal to

protected static final Pair<TokenSource, CharStream> EMPTY_SOURCE =
    new Pair<TokenSource, CharStream>(null, null);
// CommonToken.java

@Override
public String getText() {
	if ( text!=null ) {
		return text;
	}
	CharStream input = getInputStream();
	if ( input==null ) return null; // <-- oops
	int n = input.size();
	if ( start<n && stop<n) {
		return input.getText(Interval.of(start,stop));
	}
	else {
		return "<EOF>";
	}
}
// ...
public String toString(Recognizer<?, ?> r) {
	String channelStr = "";
	if ( channel>0 ) {
		channelStr=",channel="+channel;
	}
	String txt = getText();
	if ( txt!=null ) { // <-- this is what saved us in toString
		txt = txt.replace("\n","\\n");
		txt = txt.replace("\r","\\r");
		txt = txt.replace("\t","\\t");
	}
	else {
		txt = "<no text>";
	}
	String typeString = String.valueOf(type);
	if ( r!=null ) {
		typeString = r.getVocabulary().getDisplayName(type);
	}
	return "[@"+getTokenIndex()+","+start+":"+stop+"='"+txt+"'<"+typeString+">"+channelStr+","+line+":"+getCharPositionInLine()+"]";
}

Krzmbrzl and others added 3 commits April 30, 2023 14:36
The badge in the README suggests that ANTLR requires Java 7 or higher,
whereas since ANTLR v4.10 Java 11 or higher is required.

Signed-off-by: Robert Adam <dev@robert-adam.de>
Signed-off-by: Victor Smirnov <vityaman.dev@yandex.ru>
Allow user to subclass and consume differently. Useful for CR handling as suggested in antlr#2519

Signed-off-by: Alberto Simões <Alberto.Simoes@checkmarx.com>
Signed-off-by: Victor Smirnov <vityaman.dev@yandex.ru>
…g null when inputStream is null

Signed-off-by: Victor Smirnov <vityaman.dev@yandex.ru>
@vityaman vityaman force-pushed the 4078-runtime-java-to-string-tree-npe branch from 86e2795 to e25f89d Compare April 30, 2023 11:37
@@ -1,6 +1,6 @@
# ANTLR v4

[![Java 7+](https://img.shields.io/badge/java-7+-4c7e9f.svg)](http://java.oracle.com)
[![Java 11+](https://img.shields.io/badge/java-11+-4c7e9f.svg)](http://java.oracle.com)
Copy link
Member

Choose a reason for hiding this comment

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

it doesn't look a change related to the title of the current pull request.

@@ -779,7 +779,7 @@ public int Column
}


public void Consume(ICharStream input)
public virtual void Consume(ICharStream input)
Copy link
Member

Choose a reason for hiding this comment

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

It relates neither to PR nor to the Java target.

Comment on lines -64 to +67
if ( c==' ' && escapeSpaces ) buf.append('\u00B7');
else if ( c=='\t' ) buf.append("\\t");
else if ( c=='\n' ) buf.append("\\n");
else if ( c=='\r' ) buf.append("\\r");
if (c == ' ' && escapeSpaces) buf.append('\u00B7');
else if (c == '\t') buf.append("\\t");
else if (c == '\n') buf.append("\\n");
else if (c == '\r') buf.append("\\r");
Copy link
Member

Choose a reason for hiding this comment

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

Please don't mix whitespace and semantics changes (ideally, don't change whitespaces without need).

Comment on lines -63 to +66
if ( symbol.getType() == Token.EOF ) return "<EOF>";
return symbol.getText();
if (symbol.getType() == Token.EOF) {
return "<EOF>";
}
return symbol.getText();
Copy link
Member

Choose a reason for hiding this comment

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

Also, please revert this change.

@vityaman
Copy link
Author

Will close this and resolve issues in PR #4252 as here I have problems with commit history and it is easier to solve them by creating a fresh one.

@vityaman vityaman closed this Apr 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants