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

Stub limitations with Inner Classes #570

Merged
merged 10 commits into from
Jan 29, 2016
Merged

Stub limitations with Inner Classes #570

merged 10 commits into from
Jan 29, 2016

Conversation

smillst
Copy link
Member

@smillst smillst commented Jan 28, 2016

Clarifies/adds limitations as discussed in Issue #568.

or
\begin{Verbatim}
ProcessBuilder redirectError(@NonNull Redirect destination)
\end{Verbatim}
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to write the parameter's type as "ProcessBuilder.Redirect"? Showing that example, or explaining that it is not permitted to be written, would be helpful.

@smillst
Copy link
Member Author

smillst commented Jan 28, 2016

@mernst I committed the fix you asked for.

@mernst
Copy link
Member

mernst commented Jan 28, 2016

Looks good. Merge when tests pass.

\item
Annotations on types that are inner classes must be written before the
package if the type is written as a
fully annotated type, or before the inner class name if the type is
Copy link
Member

Choose a reason for hiding this comment

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

Should the "fully annotated type" be "fully qualified type"?

Annotations on types that are inner classes must be written before the
package if the type is written as a
fully-qualified type, or before the inner class name if the type is
written as a simple type. For example,
Copy link
Member

Choose a reason for hiding this comment

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

Werner says (the comment got wiped out because of a subsequent commit):
Can we mark the cases in the manual that violate Java 8 syntax, to highlight the discrepancies to source code?
Also, for each such mismatch, can we file an issue against the stub parser and reference the issue here.
(In Java 8 syntax, the example should be java.lang.ProcessBuilder.@nonnull Redirect.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we mark the cases in the manual that violate Java 8 syntax, to highlight the discrepancies to source code?

All of the stub file limitations listed in this section are violations of Java 8 syntax.

Also, for each such mismatch, can we file an issue against the stub parser and reference the issue here.

Since the right way to fix all of these limitations is to update the version of the JavaParser used by the stub parser, I don't think it makes sense to file an issue for each individual limitation. Though we could file one that says "update JavaParser".

Copy link
Member

Choose a reason for hiding this comment

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

All of the stub file limitations listed in this section are violations of Java 8 syntax.

Sure, but it might be clearer if the manual states that more explicitly.

I hadn't realized that replacing all of the StubParser was your plan. If so, I agree that having one issue is enough.

Copy link
Member

Choose a reason for hiding this comment

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

This sounds like a good resolution to me.
It definitely isn't worth fixing any StubParser problems without updating to the latest version.

Copy link
Member Author

Choose a reason for hiding this comment

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

Turns out most of the limitations already have issues ( #568, #407, #297, #220). I also submitted one for updating JavaParser #571.

@smillst
Copy link
Member Author

smillst commented Jan 28, 2016

From Werner:
I hadn't realized that replacing all of the StubParser was your plan. If so, I agree that having one issue is enough.

I thought that was generally agreed upon plan for fixing these limitations. It's not something on my agenda at the moment.

smillst added a commit that referenced this pull request Jan 29, 2016
Stub limitations with Inner Classes
@smillst smillst merged commit a54416c into master Jan 29, 2016
@smillst smillst deleted the stub-limitations branch January 29, 2016 21:28
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.

3 participants