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 157 enhance parser frontend #447

Merged
merged 8 commits into from
Sep 4, 2016
Merged

Issue 157 enhance parser frontend #447

merged 8 commits into from
Sep 4, 2016

Conversation

matozoid
Copy link
Contributor

@matozoid matozoid commented Aug 31, 2016

#157 suggests a new interface for JavaParser. This PR contains a reduced version of the suggestion.

There are two interfaces:

  1. the static methods on JavaParser form the "quick & dirty" interface
  2. the methods on an instance of JavaParser are the "power user" interface.

Interface 1 has not been changed much. There is a different exception, and that exception can hold multiple parsing problems.

For interface 2:

  • what we're parsing (compilation unit, expression, block...) is now specified as a parameter instead of using a different parameter. This happens to be a single-method interface, so now any production of ASTParser can be used.
  • where the source code is coming from is now determined by JavaCC's Provider interface. The Providers factory class has several methods for creating Providers.
  • parsing doesn't always fail on the first parsing problem anymore. It still does in many cases, but this can be improved. This way we can collect the problems and return them, instead of returning only one. Additionally the parse result, the node, is returned if parsing was completed. Both can be returned from a single parse call: a list of problems and a result node, which lays the ground work for continuous parsing (Continuous parsing. #136)

Things that are significantly different from #157:

  • There is no reference to the origin of the source code (the path() suggestion.) This can be done, but I don't know how useful it is (we can indicate a file, but how do we say that the source code came from a string or an input stream?) It can always be added on later.
  • Problems don't have a severity. I can't imagine needing it right now since everything seems to be an error. Maybe when this feature is used extensively there will be a need.

Things to do:

Thanks to @ptitjes for thinking this up!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 63.718% when pulling 804bed8 on matozoid:issue_157_enhance_parser_frontend into 183a34e on javaparser:master.

@ptitjes
Copy link
Contributor

ptitjes commented Sep 1, 2016

Wow!!!
I'm so mixed fellings: glad to finally see thing coming up together, so sad to not have been given the opportunity to make those happen...
Thanks for the kudos anyway!
(Shit, we are on par on the error reporting front now! I feel like I have to finish that error recovery very quickly...)

@matozoid matozoid mentioned this pull request Sep 1, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 63.751% when pulling e9aa743 on matozoid:issue_157_enhance_parser_frontend into 183a34e on javaparser:master.

@matozoid
Copy link
Contributor Author

matozoid commented Sep 2, 2016

@ptitjes it's funny how you suddenly started working on jlato again when I picked up development here :)

@matozoid
Copy link
Contributor Author

matozoid commented Sep 2, 2016

I just realized that if we want self-describing and rereadable providers, that the Provider interface is too bare.

@matozoid
Copy link
Contributor Author

matozoid commented Sep 2, 2016

Stuck issue #448 in here too.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 63.727% when pulling c24b525 on matozoid:issue_157_enhance_parser_frontend into 183a34e on javaparser:master.

@chmuche
Copy link

chmuche commented Sep 3, 2016

Hello, the feature look great, but I have a question, You always check if on of the method setSource is called before parsing with the method checkSourceSet so today this code break the compatibility.
So I think we can use setSource as factory method, in that case no need to check.
I think somethings like that :

/**
     * Instantiate the parser. Note that parsing can also be done with the static methods on this class.
     * Creating an instance will reduce setup time between parsing files.
     */
    private JavaParser(Provider provider) {
        this.provider= provider;
        if(astParser==null) {
            astParser = new ASTParser(provider);
        }else{
            astParser.ReInit(provider);
        }
    }

    public static JavaParser setSource(Provider provider) {
        Objects.requireNonNull(provider, "The provider can't be null");
        return new JavaParser(provider);
    }

    public static JavaParser setSource(Reader reader) {
        Objects.requireNonNull(reader, "The reader can't be null");
        return setSource(new StreamProvider(reader));
    }

    public static JavaParser setSource(InputStream input) throws IOException {
        Objects.requireNonNull(input, "The InputStream can't be null");
        return setSource(new StreamProvider(input));
    }

    public static JavaParser setSource(InputStream input, Charset encoding) throws IOException {
        if (input == null){
            throw new IOException("The InputStream can't be null");
        }
        Objects.requireNonNull(encoding, "The encoding can't be null");
        return setSource(new StreamProvider(input, encoding.name()));
    }

    public static JavaParser setSource(File file) throws IOException {
        Objects.requireNonNull(file, "The file can't be null");
        return setSource(new FileInputStream(file));
    }

    public static JavaParser setSource(File file, Charset encoding) throws IOException {
        Objects.requireNonNull(file, "The file can't be null");
        Objects.requireNonNull(encoding, "The encoding can't be null");
        return setSource(new FileInputStream(file), encoding);
    }

    public static JavaParser setSource(String source) {
        Objects.requireNonNull(source, "The source can't be null");
        return setSource(new StringReader(source));
    }

* Factory for providers of source code for JavaParser.
* Providers that have no parameter for encoding but need it will use UTF-8.
*/
public abstract class Providers {
Copy link

Choose a reason for hiding this comment

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

Why abstract and not final ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh woops...

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 63.727% when pulling 1c09385 on matozoid:issue_157_enhance_parser_frontend into 183a34e on javaparser:master.

/**
* Thrown when parsing problems occur during parsing with the static methods on JavaParser.
*/
public class ParseProblemException extends Exception {
Copy link

Choose a reason for hiding this comment

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

Me when I code I prefere to create RuntimeException, Or using an another Pattern because with exception we can't do composition.If you want to talk about, PM me on gitter 😄 otherwise it's fine 😸

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nah, I think I agree, it's the Java 8 world and we want to use the functional programming features, and checked exceptions are hated there.

Copy link
Member

Choose a reason for hiding this comment

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

Yep I agree, checked exceptions are annoying :p

Le sam. 3 sept. 2016 à 17:21, Danny van Bruggen notifications@github.com
a écrit :

In
javaparser-core/src/main/java/com/github/javaparser/ParseProblemException.java
#447 (comment):

@@ -0,0 +1,24 @@
+package com.github.javaparser;
+
+import java.util.List;
+
+/**

  • * Thrown when parsing problems occur during parsing with the static methods on JavaParser.
  • */
    +public class ParseProblemException extends Exception {

Nah, I think I agree, it's the Java 8 world and we want to use the
functional programming features, and checked exceptions are hated there.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/javaparser/javaparser/pull/447/files/c24b5256f3ebdb0903b48fc0c77106904658a525#r77438130,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AJu56OiXlQlQsxaWpQI5-6ByV-jskUIkks5qmZCKgaJpZM4JyGiu
.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 63.727% when pulling 92c384a on matozoid:issue_157_enhance_parser_frontend into 183a34e on javaparser:master.

@matozoid
Copy link
Contributor Author

matozoid commented Sep 3, 2016

@chmuche - I don't really get your other comment. Did you notice that setSource is gone? It was only there in alpha-1 and wasn't a great design, introducing state where it shouldn't have been.

I like Objects.requireNotNull, will make an issue for discussing nullity stuff.

@matozoid
Copy link
Contributor Author

matozoid commented Sep 4, 2016

Needs more work on exception handling, but this gets merged right now because it is causing merge problems.

# Conflicts:
#	javaparser-core/src/main/java/com/github/javaparser/ast/visitor/ModifierVisitorAdapter.java
@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 63.014% when pulling 838660c on matozoid:issue_157_enhance_parser_frontend into 5a534de on javaparser:master.

@matozoid matozoid merged commit ead1412 into javaparser:master Sep 4, 2016
@matozoid matozoid deleted the issue_157_enhance_parser_frontend branch September 4, 2016 19:21
@matozoid matozoid added this to the next_release milestone Sep 4, 2016
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.

5 participants