[proposal] check packages names #70

guiguilechat · 2019-09-28T13:29:58Z

Issue

The JDefinedClass constructor checks the validity of the class name, starting here :

https://github.com/phax/jcodemodel/blob/master/src/main/java/com/helger/jcodemodel/JDefinedClass.java#L214

The JPackage constructor, does not, here :

https://github.com/phax/jcodemodel/blob/master/src/main/java/com/helger/jcodemodel/JPackage.java#L116

Proposal

I propose to make the JPackage make a verification test

https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html

according to this :

sName must not be a reserved keyword
sName must not contain non-alphanumerical value besides the underscore
sName must not start with a number.
(optional) sName must not contain an upper-case character.

Implementation

Here is how I personally do :

Reserved keyword

I keep a list from https://docs.oracle.com/javase/tutorial/java/nutsandbolts/_keywords.html

/** java keywords we can't use as a name for a package or a class */
public static final Set<String> JAVA_RESERVED_KEYWORDS = Collections.unmodifiableSet(new HashSet<>(
		Arrays.asList("abstract", "assert", "boolean", "break", "byte", "case", "catch", "char", "class", "const",
				"continue", "default", "do", "double", "else", "extends", "false", "final", "finally", "float", "for", "goto",
				"if", "implements", "import", "instanceof", "int", "interface", "long", "native", "new", "null", "package",
				"private", "protected", "public", "return", "short", "static", "strictfp", "super", "switch", "synchronized",
				"this", "throw", "throws", "transient", "true", "try", "void", "volatile", "while")));

Then I just check if the name is contained in that set.

All alphanumerical, start with non-numerical

public static final Pattern VALID_PACKAGE_NAME = Pattern.compile("[A-Za-z_][A-Za-z0-9_]*");

Then I just check if sName matches this pattern. It can accept "_" as a package name, not sure if correct.

Optional check lowercase

Add a boolean option in JCodeModel::ensurePackagesLowerCase(). default value should be false.
Then create a new Pattern to match the package name

public static final Pattern VALID_LOWERCASE_PACKAGE_NAME = Pattern.compile("[a-z_][a-z0-9_]*");

To test the name against.

The text was updated successfully, but these errors were encountered:

guiguilechat · 2019-09-28T14:01:57Z

Just saying, this is actually an issue because it generates invalid java code.
Which is then much harder to debug. In my case I had a web API with a resource whose resource path contained assets[raw] or something like that. This crashed much later in the compilation process, when I needed to access the generated classes.

Of course it is not an issue if we accept to generate invalid package names, but then it's an inconsistency because the classes do not allow invalid class names.

phax · 2019-09-30T09:41:53Z

@guiguilechat Thanks for the very verbose request :)
I agree, it makes sense.
The keyword list is already present in class JJavaName.
I will make the suggestion for lower case optional for backwards compatibility.

glelouet · 2019-09-30T18:22:15Z

if (isForbiddenPackageNamePart (sPart))
throw new IllegalArgumentException ("The package name '" + sName + "' is invalid");
}

should print the sPart : "The subpackage name " +sPart+" is invalid in package "+sName

??

glelouet · 2019-09-30T18:26:19Z

// May not be a keyword
try
{
  aCM._package ("org.example.var");
  fail ();
}
catch (final IllegalArgumentException ex)
{}

var is not a reserved keyword.

public static void var.MyClass::main(){var var = 12;}

is correct (just checked)

glelouet · 2019-09-30T18:31:15Z

sorry I have issues switching accounts.

BTW you can just add "fixed #70" or "fix #70" in the commit and it will auto close the issue ;)

glelouet · 2019-09-30T18:53:27Z

Let me tell you, sir, about our lord and savior maven.

Release plugin

I would advise you to use the maven release module

mvn clean release:prepare release:perform

which automatically changes the versions, while still working on the snapshot .

However there are a few issues, eg it may break because compilation phases have non-deterministic effect (don't use the mvn clean in that case : instead make two separate calls mvn clean install && mvn release:prepare release:perform .

Other issues is that you may crash and add a tag in local and remote that will later prevent you from actually updating the value to the next version. In that case, delete the tag from local and remote repository with eg mvn release:rollback; git tag -d 1.2.3 && git push --delete origin 1.2.3 if you were at the version 1.2.3-SNAPSHOT before release and you asked to release the 1.2.3 when it failed.

You also need to set your csm to a correct value. Here is mine for Jswagermaker :

<scm>
	<url>https://github.com/glelouet/JSwaggerMaker</url>
	<connection>scm:git:git@github.com:glelouet/JSwaggerMaker</connection>
      <developerConnection>scm:git:git@github.com:glelouet/JSwaggerMaker</developerConnection>
	<tag>HEAD</tag>
</scm>

and the distributionManagement part .

skip javadoc

Also you can ask mvn to not create the javadoc (a single property in the root pom)
Here are a few properties I often use :

<properties>
	<maven.javadoc.skip>true</maven.javadoc.skip>
</properties>

I think it is un necessary when you already export the sources ^^

specify maven version and encoding

<properties>
	<java.version>11</java.version>
	<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>

java.version is used in the maven-compiler-plugin to enforce the compilation against a specific version.

<build>
	<plugins>
		<plugin>
			<artifactId>maven-compiler-plugin</artifactId>
			<version>3.7.0</version>
			<configuration>
				<release>${java.version}</release>
			</configuration>
		</plugin>

encoding is specified to be sure everybody gets the code correctly even mac users :)

Attach sources on deploy

use the plugin, Luke :

<build>
	<plugins>
		<plugin>
			<artifactId>maven-source-plugin</artifactId>
			<version>3.1.0</version>
			<executions>
				<execution>
					<id>attach-sources</id>
					<goals>
						<!-- -no-fork prevents from compiling twice -->
						<goal>jar-no-fork</goal>
					</goals>
				</execution>
			</executions>
		</plugin>

as written no-fork prevents from doing it both in the compile and release parts or something like that.

Export to an ftp server

you still need the server configuration in your LOCAL pom.

	<extensions>
		<!-- Enabling the use of FTP -->
		<extension>
			<groupId>org.apache.maven.wagon</groupId>
			<artifactId>wagon-ftp</artifactId>
		</extension>
	</extensions>

if it crashes, you need to specify the plugin version in the dependency management.(yeah sometimes maven sucks)

phax · 2019-09-30T21:01:17Z

haha. Okay, will take 'var' off the list again. And of course I know the release plugin. Give me some time to check the other proposal first ;)

phax · 2019-09-30T21:06:51Z

Part of 3.3.0 release

glelouet · 2019-09-30T21:22:27Z

Sorry I talking about maven because I did not notice a "maven-release" in the commits. Just trying to ease the deployment. Also because I had several colleagues that were very good, just not with maven - and therefore did not eg deploy their sources at the same time they did a release.

according to
https://docs.oracle.com/javase/specs/jls/se12/html/jls-3.html#jls-Identifier
"The underscore may be used in identifiers formed of two or more characters, but it cannot be used as a one-character identifier due to being a keyword. "

so "_" is not a valid package identifier. This answers my previous question. It is missing from the reserved keywords list

jcodemodel/src/main/java/com/helger/jcodemodel/JJavaName.java

Line 112 in 6696b9e

/*

Just checked and you added true, false, null, so it's good.

Also interesting is
" Type identifiers are used in certain contexts involving the declaration or use of types. For example, the name of a class must be a TypeIdentifier, so it is illegal to declare a class named var (§8.1). ". So "var" is allowed for var name, package identifier, but not for class names.

Please take all your time :) Sorry I did not intend to rush you.

glelouet · 2019-09-30T21:33:53Z

should I make a different issue to enhance maven ?

phax · 2019-10-01T05:57:20Z

So the single underscore is resolved.
All the Maven things that you are referring to are part of the paren POM at https://github.com/phax/ph-parent-pom - so if you are missing something there - please file an issue there.
The SNAPSHOTs are pushed via Travis to Maven Central SNAPSHOT repo - https://oss.sonatype.org/content/repositories/snapshots/com/helger/jcodemodel/ - so no need to FTP/SFTP.
I'm building all my stuff for Java 8+ and for that, the setup is quite OK I guess.

glelouet · 2019-10-01T10:04:19Z

actually I think I'm gonna take some things from there … :)

fbaro · 2019-12-30T11:26:41Z

@phax I'm updating to the latest release, and I cannot generate resource files in the META-INF package because of this bug fix. In particular, I was generating files in META-INF/services. I poked around a bit, but could not find any other way to do this. Did I miss something?

guiguilechat · 2019-12-30T11:35:14Z

@fbaro please provide an example and explain how it is not working.

fbaro · 2019-12-30T11:41:44Z

Well when I call codeModel.rootPackage().subPackage("META-INF"), I get this exception

Caused by: java.lang.IllegalArgumentException: Part 'META-INF' of the package name 'META-INF' is invalid
	at com.helger.jcodemodel.JPackage.<init>(JPackage.java:194)
	at com.helger.jcodemodel.JCodeModel.lambda$_package$0(JCodeModel.java:226)
	at java.util.HashMap.computeIfAbsent(HashMap.java:1127)
	at com.helger.jcodemodel.JCodeModel._package(JCodeModel.java:226)
	at com.helger.jcodemodel.JPackage.subPackage(JPackage.java:411)

guiguilechat · 2019-12-30T11:50:37Z

https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html

In some cases, (name) may not be a valid package name. This can occur if the domain name contains a hyphen or other special character(…). In this event, the suggested convention is to add an underscore.

Also,

Package names are written in all lower case to avoid conflict with the names of classes or interfaces.

therefore, the correct package name should be meta_inf

However, if you are creating resources, it's a different issue. But why are you using jcodemodel to generate resources ? Do you have a link to a working example ?

fbaro · 2019-12-30T14:59:14Z

I'm using JCodeModel to generate a lot of java code, and also some plain text files. I do it because it's possible and supported by the JCodeModel interface (JPackage.addResourceFile).

That name is correct as it is, and cannot be changed. See https://docs.oracle.com/javase/7/docs/api/java/util/ServiceLoader.html

I'm unable to link to a working example. However the line of code above is sufficient to trigger the exception, no further code is needed.

guiguilechat · 2019-12-30T17:19:59Z

That name is correct for the "resource directory", not for a package.

If I understand correctly, your issue is that there is no way anymore to create a resource in the directory "META-INF" .

@phax I think there should be a separation between resource directories, and packages. With the ability to go from a package to its corresponding resource dir with a asResDir() .

create the JResourceDirectory class (same level as JPackage)
move the method addResourceFile from JPackage to JResourceDirectory
add sub-dir creation in this class, and the root JResourceDirectory in JCodemodel
add JPackage to JResourceDir translation.

phax · 2020-01-03T21:50:35Z

There is a solution in ph-jdmc - details after my vacation next week

phax self-assigned this Sep 28, 2019

phax added the enhancement label Sep 28, 2019

guiguilechat mentioned this issue Sep 28, 2019

[proposal] check class name against keywords and java.lang #71

Closed

phax added a commit that referenced this issue Sep 30, 2019

Added package name checks; #70

8f06931

phax added a commit that referenced this issue Sep 30, 2019

var is not a keyword; #70

6696b9e

phax added a commit that referenced this issue Sep 30, 2019

Improved error message; #70

6a416e1

phax added a commit that referenced this issue Sep 30, 2019

Improved test #70

cb9ef66

phax closed this as completed Sep 30, 2019

phax added a commit that referenced this issue Oct 1, 2019

Added single underscore; #70

79cbb36

guiguilechat mentioned this issue Jan 3, 2020

create resource directory #74

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[proposal] check packages names #70

[proposal] check packages names #70

guiguilechat commented Sep 28, 2019 •

edited

Loading

guiguilechat commented Sep 28, 2019

phax commented Sep 30, 2019

glelouet commented Sep 30, 2019 •

edited

Loading

glelouet commented Sep 30, 2019 •

edited

Loading

glelouet commented Sep 30, 2019

glelouet commented Sep 30, 2019

phax commented Sep 30, 2019

phax commented Sep 30, 2019

glelouet commented Sep 30, 2019 •

edited

Loading

glelouet commented Sep 30, 2019

phax commented Oct 1, 2019

glelouet commented Oct 1, 2019

fbaro commented Dec 30, 2019

guiguilechat commented Dec 30, 2019

fbaro commented Dec 30, 2019

guiguilechat commented Dec 30, 2019 •

edited

Loading

fbaro commented Dec 30, 2019

guiguilechat commented Dec 30, 2019

phax commented Jan 3, 2020

[proposal] check packages names #70

[proposal] check packages names #70

Comments

guiguilechat commented Sep 28, 2019 • edited Loading

Issue

Proposal

Implementation

Reserved keyword

All alphanumerical, start with non-numerical

Optional check lowercase

guiguilechat commented Sep 28, 2019

phax commented Sep 30, 2019

glelouet commented Sep 30, 2019 • edited Loading

glelouet commented Sep 30, 2019 • edited Loading

glelouet commented Sep 30, 2019

glelouet commented Sep 30, 2019

Release plugin

skip javadoc

specify maven version and encoding

Attach sources on deploy

Export to an ftp server

phax commented Sep 30, 2019

phax commented Sep 30, 2019

glelouet commented Sep 30, 2019 • edited Loading

glelouet commented Sep 30, 2019

phax commented Oct 1, 2019

glelouet commented Oct 1, 2019

fbaro commented Dec 30, 2019

guiguilechat commented Dec 30, 2019

fbaro commented Dec 30, 2019

guiguilechat commented Dec 30, 2019 • edited Loading

fbaro commented Dec 30, 2019

guiguilechat commented Dec 30, 2019

phax commented Jan 3, 2020

guiguilechat commented Sep 28, 2019 •

edited

Loading

glelouet commented Sep 30, 2019 •

edited

Loading

glelouet commented Sep 30, 2019 •

edited

Loading

glelouet commented Sep 30, 2019 •

edited

Loading

guiguilechat commented Dec 30, 2019 •

edited

Loading