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

Issue #11166: remove getFileContents from EmptyLineSeparator #14819

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mahfouz72
Copy link
Member

@mahfouz72 mahfouz72 commented Apr 21, 2024

Issue #11166 :
refactor EmptyLineSeparatorCheck to use AST-based logic not getFileContents()

Diff Regression config: https://gist.githubusercontent.com/mahfouz72/afd57aa6e51ca161b7927acd959e5211/raw/9384f843297bb2e10c074bcb33ddc46902d6bd52/check.xml

WIP. just wants to get an initial review from CIs and maintainers

@mahfouz72
Copy link
Member Author

mahfouz72 commented Apr 21, 2024

dealing with comment tokens in the AST was weird to me. why it is under modifiers of the next subtree. is there a reason to build AST like this?

  int x;
    // m
     public int m() {
        return 0;
    }
 |--VARIABLE_DEF -> VARIABLE_DEF [4:4]
        |   |--MODIFIERS -> MODIFIERS [4:4]
        |   |--TYPE -> TYPE [4:4]
        |   |   `--LITERAL_INT -> int [4:4]
        |   |--IDENT -> x [4:8]
        |   `--SEMI -> ; [4:9]
        |--METHOD_DEF -> METHOD_DEF [6:3]
        |   |--MODIFIERS -> MODIFIERS [6:3]
        |   |   |--SINGLE_LINE_COMMENT -> // [5:4]
        |   |   |   `--COMMENT_CONTENT ->  m\r\n [5:6]
        |   |   `--LITERAL_PUBLIC -> public [6:3]


@romani @nrmancuso @rnveach
do we have any check that is aware of comments using AST-based logic not file content to take it as a reference?

@mahfouz72 mahfouz72 marked this pull request as draft April 21, 2024 18:06
Comment on lines +697 to +707
private DetailAST findFirstCommentToken(DetailAST ast) {
final DetailAST modifersAst = ast.findFirstToken(TokenTypes.MODIFIERS);
final DetailAST commentAst;

if (modifersAst != null) {
commentAst = modifersAst.getFirstChild();
}
else {
commentAst = ast.getFirstChild();
}
return commentAst;
Copy link
Member Author

Choose a reason for hiding this comment

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

I will work more on this method. I believe that there are more cases that need to be handled to get the first comment token

@mahfouz72
Copy link
Member Author

Github, generate report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants