You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/07/24 14:34:29 UTC
[GitHub] [maven-jxr] sman-81 opened a new pull request, #60: [JXR-170] NullPointerException while parsing Java 15 multi-line String source
sman-81 opened a new pull request, #60:
URL: https://github.com/apache/maven-jxr/pull/60
This PR offers a fix for bug [JXR-170]. Summary of changes:
- Add support for parsing sources with Java 15 multi-line Strings
- Add JUnit test case
- Implement toString on class BaseType
- Require non-null name on BaseType construtor
- Prevent construction of types with null name
- Add version to maven-enforcer-plugin plugin to prevent failure of integration test JXR-143_nofork
Following this checklist to help us incorporate your contribution quickly and easily:
- [x] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/JXR) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [x] Each commit in the pull request should have a meaningful subject line and body.
- [x] Format the pull request title like `[JXR-XXX] - Fixes bug in ApproximateQuantiles`,
where you replace `JXR-XXX` with the appropriate JIRA issue. Best practice
is to use the JIRA issue title in the pull request title and in the first line of the
commit message.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [x] You have run the integration tests successfully (`mvn -Prun-its clean verify`).
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [x] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jxr] sman-81 commented on a diff in pull request #60: [JXR-170] NullPointerException while parsing Java 15 multi-line String source
Posted by GitBox <gi...@apache.org>.
sman-81 commented on code in PR #60:
URL: https://github.com/apache/maven-jxr/pull/60#discussion_r928523977
##########
maven-jxr/src/main/java/org/apache/maven/jxr/pacman/BaseType.java:
##########
@@ -36,22 +48,15 @@
*/
public String getName()
{
- if ( name == null )
- {
- return "";
- }
return this.name;
}
- /**
- * Set the name for this type
- *
- * @param name The new name value
- */
- public void setName( String name )
+ @Override
+ public String toString()
{
- this.name = name;
+ return getClass().getSimpleName() + "[name=" + name + "]";
Review Comment:
It's good practice to provide `toString` implementations.
The toString() method is used to represent objects as human-readable text.
Great help in debugging!
Square brackets are widely accepted (e.g. the toString template of Eclipse uses them).
> format [...] can be misunderstood as array
and curly braces can be misunderstood as Map ;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jxr] slawekjaranowski commented on pull request #60: [JXR-170] NullPointerException while parsing Java 15 multi-line String source
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #60:
URL: https://github.com/apache/maven-jxr/pull/60#issuecomment-1193760692
@sman-81 thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jxr] sman-81 commented on a diff in pull request #60: [JXR-170] NullPointerException while parsing Java 15 multi-line String source
Posted by GitBox <gi...@apache.org>.
sman-81 commented on code in PR #60:
URL: https://github.com/apache/maven-jxr/pull/60#discussion_r928518796
##########
maven-jxr-plugin/src/it/JXR-143_nofork/pom.xml:
##########
@@ -44,6 +44,7 @@ under the License.
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
+ <version>3.1.0</version>
Review Comment:
Are you aware this integration test `JXR-143_nofork` fails on master as well?
The project requires that the integration tests run successfully (`mvn -Prun-its clean verify`).
So I fixed it.
The root cause is failure to find `maven-enforcer-plugin`.
After adding a plugin version, the test is green.
I've looked at the test however: the usage of maven-enforcer-plugin looks like non-sense, rules = AlwaysPass ;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jxr] slawekjaranowski commented on a diff in pull request #60: [JXR-170] NullPointerException while parsing Java 15 multi-line String source
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #60:
URL: https://github.com/apache/maven-jxr/pull/60#discussion_r928579341
##########
maven-jxr-plugin/src/it/JXR-143_nofork/pom.xml:
##########
@@ -44,6 +44,7 @@ under the License.
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
+ <version>3.1.0</version>
Review Comment:
Current build on CI on master pass as I see ...
rule `Always Pass` probably was added in order to override rules from parent ... I will check it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jxr] slawekjaranowski merged pull request #60: [JXR-170] NullPointerException while parsing Java 15 multi-line String source
Posted by GitBox <gi...@apache.org>.
slawekjaranowski merged PR #60:
URL: https://github.com/apache/maven-jxr/pull/60
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jxr] sman-81 commented on pull request #60: [JXR-170] NullPointerException while parsing Java 15 multi-line String source
Posted by GitBox <gi...@apache.org>.
sman-81 commented on PR #60:
URL: https://github.com/apache/maven-jxr/pull/60#issuecomment-1193762468
> @sman-81 thanks
Thank you, too @slawekjaranowski
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jxr] slawekjaranowski commented on a diff in pull request #60: [JXR-170] NullPointerException while parsing Java 15 multi-line String source
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #60:
URL: https://github.com/apache/maven-jxr/pull/60#discussion_r928270393
##########
maven-jxr/src/test/java/org/apache/maven/jxr/pacman/JavaFileImplTest.java:
##########
@@ -46,4 +46,14 @@ public void testJXR_135_lotsOfNested() throws IOException
assertEquals( "NotNested", classTypes.next().getName() );
}
-}
\ No newline at end of file
+ @Test
+ public void testJXR_170_multiLineString() throws IOException
+ {
+ JavaFileImpl javaFile = new JavaFileImpl( Paths.get(
+ "src/test/resources/jxr170/org/apache/maven/jxr/pacman/ClassWithMultiLineString.java" ),
+ "UTF-8" );
+ assertEquals( 1, javaFile.getClassTypes().size() );
+ assertEquals( "ClassWithMultiLineString", javaFile.getClassTypes().get(0).getName() );
+ assertEquals( "[ImportType[name=java.lang.*]]", javaFile.getImportTypes().toString() );
Review Comment:
if `getImportTypes` - return array / list pease check similarly like for `getClassTypes`
##########
maven-jxr/src/main/java/org/apache/maven/jxr/pacman/BaseType.java:
##########
@@ -19,15 +19,27 @@
* under the License.
*/
+import java.util.Objects;
+
/**
* put your documentation comment here
*
* @author jvanzyl
*/
public abstract class BaseType
{
- private String name = null;
+ private final String name;
+ /**
+ * Construct type and set its name.
+ *
+ * @param name type name
+ */
+ public BaseType( String name )
+ {
+ Objects.requireNonNull( name );
+ this.name = name;
Review Comment:
```
this.name = Objects.requireNonNull( name );
```
##########
maven-jxr-plugin/src/it/JXR-143_nofork/pom.xml:
##########
@@ -44,6 +44,7 @@ under the License.
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
+ <version>3.1.0</version>
Review Comment:
How connected with this change?
##########
maven-jxr/src/main/java/org/apache/maven/jxr/pacman/BaseType.java:
##########
@@ -36,22 +48,15 @@
*/
public String getName()
{
- if ( name == null )
- {
- return "";
- }
return this.name;
}
- /**
- * Set the name for this type
- *
- * @param name The new name value
- */
- public void setName( String name )
+ @Override
+ public String toString()
{
- this.name = name;
+ return getClass().getSimpleName() + "[name=" + name + "]";
Review Comment:
where is used?
format `[...]` can be misunderstood as array
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jxr] sman-81 commented on a diff in pull request #60: [JXR-170] NullPointerException while parsing Java 15 multi-line String source
Posted by GitBox <gi...@apache.org>.
sman-81 commented on code in PR #60:
URL: https://github.com/apache/maven-jxr/pull/60#discussion_r928515378
##########
maven-jxr/src/test/java/org/apache/maven/jxr/pacman/JavaFileImplTest.java:
##########
@@ -46,4 +46,14 @@ public void testJXR_135_lotsOfNested() throws IOException
assertEquals( "NotNested", classTypes.next().getName() );
}
-}
\ No newline at end of file
+ @Test
+ public void testJXR_170_multiLineString() throws IOException
+ {
+ JavaFileImpl javaFile = new JavaFileImpl( Paths.get(
+ "src/test/resources/jxr170/org/apache/maven/jxr/pacman/ClassWithMultiLineString.java" ),
+ "UTF-8" );
+ assertEquals( 1, javaFile.getClassTypes().size() );
+ assertEquals( "ClassWithMultiLineString", javaFile.getClassTypes().get(0).getName() );
+ assertEquals( "[ImportType[name=java.lang.*]]", javaFile.getImportTypes().toString() );
Review Comment:
What do you mean?
getImportTypes returns a Set, getClassTypes returns a List (this is the original code unchanged by me)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jxr] sman-81 commented on a diff in pull request #60: [JXR-170] NullPointerException while parsing Java 15 multi-line String source
Posted by GitBox <gi...@apache.org>.
sman-81 commented on code in PR #60:
URL: https://github.com/apache/maven-jxr/pull/60#discussion_r930668016
##########
maven-jxr-plugin/src/it/JXR-143_nofork/pom.xml:
##########
@@ -44,6 +44,7 @@ under the License.
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
+ <version>3.1.0</version>
Review Comment:
> Current build on CI on master pass as I see ...
>
> rule `Always Pass` probably was added in order to override rules from parent ... I will check it.
Hello @slawekjaranowski
I looked further into this. While running the integration tests I was behind a proxy with access only to an internal Nexus artifact manager. As the plugin has not version specified, the list of versions is fetched from Central - which failed.
Therefore, strictly speaking the version is not required on this plugin for the test to be green, but explicit versions are good practice and strongly encouraged since at least Maven 3.
Maybe removing `maven-enforcer-plugin` from this test altogether is the best resolution?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org