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

Don't include declarations in usages (fixes #629). #649

Merged

Conversation

ris58h
Copy link
Contributor

@ris58h ris58h commented Jul 7, 2023

No description provided.

@ris58h
Copy link
Contributor Author

ris58h commented Jul 19, 2023

@bjansen Sorry for bothering you. I don't know who should I ping to approve the workflow.

@ris58h ris58h force-pushed the 629-do-not-include-declaration-in-usages branch from 7227cbc to 01ba5a6 Compare July 19, 2023 10:09
@bjansen
Copy link
Collaborator

bjansen commented Sep 13, 2023

I'll look into it this week, sorry for the delay

@@ -29,6 +29,7 @@ public class ANTLRv4ASTFactory extends ASTFactory {
ruleElementTypeToPsiFactory.put(ANTLRv4TokenTypes.RULE_ELEMENT_TYPES.get(ANTLRv4Parser.RULE_modeSpec), ModeSpecNode.Factory.INSTANCE);
ruleElementTypeToPsiFactory.put(ANTLRv4TokenTypes.RULE_ELEMENT_TYPES.get(ANTLRv4Parser.RULE_action), AtAction.Factory.INSTANCE);
ruleElementTypeToPsiFactory.put(ANTLRv4TokenTypes.RULE_ELEMENT_TYPES.get(ANTLRv4Parser.RULE_identifier), TokenSpecNode.Factory.INSTANCE);
ruleElementTypeToPsiFactory.put(ANTLRv4TokenTypes.RULE_ELEMENT_TYPES.get(ANTLRv4Parser.RULE_tokensSpec), TokensSpecNode.Factory.INSTANCE);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure we need this, token definitions already have a parent of type TokenSpecNode:
image
So using getParent() instanceof TokenSpecNode should be enough.


public class LexerRuleRefNode extends GrammarElementRefNode {
public LexerRuleRefNode(IElementType type, CharSequence text) {
super(type, text);
}

@Override
public PsiReference getReference() {
return new GrammarElementRef(this, getText());
protected boolean isDeclaration() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would have kept getReference() here and simply add this snippet:

		if (getParent() instanceof LexerRuleSpecNode || getParent() instanceof TokenSpecNode) {
			// A declaration doesn't need to reference itself
			return null;
		}

(same in ParserRuleRefNode)

@ris58h ris58h force-pushed the 629-do-not-include-declaration-in-usages branch from 01ba5a6 to 9cb8554 Compare September 14, 2023 13:26
@ris58h
Copy link
Contributor Author

ris58h commented Sep 14, 2023

@bjansen I've made the requested changes.

Upd: I've also added ChannelSpecNode check for LexerRuleRefNode.

@bjansen bjansen added this to the 1.22 milestone Sep 14, 2023
@bjansen bjansen merged commit bcc62fa into antlr:master Sep 14, 2023
6 checks passed
@bjansen
Copy link
Collaborator

bjansen commented Sep 14, 2023

Thanks for the contribution!

@ris58h ris58h deleted the 629-do-not-include-declaration-in-usages branch September 14, 2023 20:15
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

2 participants