Skip to content

[core] Improve Parser interface#3085

Merged
oowekyala merged 35 commits into
pmd:pmd/7.0.xfrom
oowekyala:core-parser-tasks
Feb 13, 2021
Merged

[core] Improve Parser interface#3085
oowekyala merged 35 commits into
pmd:pmd/7.0.xfrom
oowekyala:core-parser-tasks

Conversation

@oowekyala

@oowekyala oowekyala commented Jan 27, 2021

Copy link
Copy Markdown
Member

Describe the PR

This is the next step for pmd 7.

  • Replace the parameters Parser#parse with a ParserTask object. This contains all the info that is passed to the parser, and may be evolved in the future. Parser is now a functional interface.
  • ParserOptions are removed. Their only remaining use, as a container for properties, is shifted onto a PropertySource that the ParserTask carries. This is transitional as they'll be converted to language properties
  • The Parser interface is moved to the nspmd.lang.ast package instead of just lang, AbstractParser is removed as it's now useless
  • Nodes have access to their language version and the name of the file. These new methods are meant to replace the statefulness of RuleContext, which we can remove in a future PR
    • This is meant to be replaced by TextDocuments in a future PR. The info is contained in an AstInfo object that is set on the root node, that way, it can also be evolved with fewer code changes.
  • Move pmd-java's SemanticChecksLogger into pmd-core. ParserTasks contain one such object, the parser may use it to validate the produced trees. This is not entirely implemented, we use a noop for now.
  • Introduce a supertype for PMD's exceptions: FileAnalysisException. These are the exceptions that may be thrown by a SourceCodeProcessor, eg ParseException or TokenMgrError.
  • Now SourceCodeProcessor reads the whole file into a string before starting the parser (the string is passed through the ParserTask). That way, the implementation of Parser does not need to deal with IOExceptions.

So, most of this has holes that need to be filled-in later, but I think it's necessary groundwork.

Related issues

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by travis)
  • Added (in-code) documentation (if needed)

@oowekyala oowekyala added this to the 7.0.0 milestone Jan 27, 2021
@oowekyala oowekyala changed the title Core parser tasks [core] Improve Parser interface Jan 27, 2021
@ghost

ghost commented Jan 27, 2021

Copy link
Copy Markdown
2 Messages
📖 Compared to pmd/7.0.x:
This changeset changes 0 violations,
introduces 0 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 1 errors and 0 configuration errors.
Full report
📖 Compared to master:
This changeset changes 1606 violations,
introduces 4187 new violations, 1 new errors and 0 new configuration errors,
removes 6137 violations, 10 errors and 2 configuration errors.
Full report

Generated by 🚫 Danger

@oowekyala

Copy link
Copy Markdown
Member Author

@adangel I think I'm going to merge this pr this weekend, and then merge the 7.0.x branch back into #2830

@oowekyala oowekyala merged commit 5ca13f1 into pmd:pmd/7.0.x Feb 13, 2021
@oowekyala oowekyala deleted the core-parser-tasks branch February 13, 2021 20:09
*
* @author Pieter_Van_Raemdonck - Application Engineers NV/SA - www.ae.be
*/
public interface Parser {

@adangel adangel Feb 19, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We could keep a interface at the old package to make it easier to transition:

package net.sourceforge.pmd.lang;

@Deprecated
@DeprecatedUntil700
public interface Parser extends net.sourceforge.pmd.lang.ast.Parser {
}

Not sure, if that's worth. Regarding these @DeprecatedUntil700 - we could move these files into a own module called "pmd-compat6". When moving from pmd6 to pmd7, this could be added as an additional dependency to help getting adjusting the code to pmd7. We could move this module later out of pmd7 (before we release it) and maintain it for a while a separately. I don't know, whether this is feasible (that you can take a rule/language impl for pmd6, add this pmd-compat6 dependency, and run it with pmd7) or whether that's worth at all...

* An error thrown during lexical analysis of a file.
*/
public final class TokenMgrError extends RuntimeException {
public final class TokenMgrError extends FileAnalysisException {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

At some point, we should probably rename that from Error to Exception...

import net.sourceforge.pmd.lang.swift.ast.SwiftParser.SwTopLevel;

// package private base class
abstract class SwiftRootNode extends SwiftInnerNode implements RootNode {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was wondering, how/when this new class is actually used. But figured out -> antlr4-wrapper.xml

@adangel adangel mentioned this pull request Feb 19, 2021
4 tasks
@oowekyala oowekyala mentioned this pull request Dec 17, 2021
5 tasks
@adangel adangel mentioned this pull request Jan 23, 2023
55 tasks
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.

2 participants