javaparser / javaparser

Java 1-21 Parser and Abstract Syntax Tree for Java with advanced analysis functionalities.
https://javaparser.org
Other
5.38k stars 1.14k forks source link

Introduce special check for package-info.java #1681

Open amriteya opened 6 years ago

amriteya commented 6 years ago

The package-info.java is a special Java file that can be added to any Java source package. Its purpose is to provide a home for package level documentation and package level annotations.

It'd be nice to have a check if the given Java file is a package-info.java file. This method could be added in CompilationUnit built on parsing the Java file.

Here is a documentation from JLS defining the package-info.java and what constitutes package-info.java

matozoid commented 6 years ago

Hi! We could add a method like "isPackageInfo" on the CompilationUnit that checks if the filename in compilationUnit, getStorage, getFilename is "package-info.java". The only question is whether it should throw an exception or return false or Optional.empty when no storage information is available. (It's only available when a file has been parsed from disk.)

A prettier alternative altogether would be to check if there are annotations and/or documentation present on the package declaration... but it seems that is not required, so it's not failsafe... but it would work when no storage information is available.

What approach would you suggest, @amriteya ?

amriteya commented 6 years ago

Well, this is an interesting problem statement.

As you mentioned, the latter alternative cannot be a reliable way to distinguish a package-info.java file. The JLS documentation clearly states that even though it is a bad practice to have anything more than package declarations and annotations declaration in the package-info.java, it is allowed to have source code there as well.

On the other hand, if we tackle the problem only using the filename, the CompilationUnit won't be consistent for different Storage types.

I guess I did not see the bigger problem here and I was only focussed on solution pertaining to filesystem based Java files.

@matozoid It's best the contributors take a decision on this as no solution here could be a 100% solution until someone else wants to pitch in a better idea of solving this.

matozoid commented 6 years ago

@amriteya alright, let's leave it open then.

matozoid commented 6 years ago

I'd better take the easy label off ;-)

malteskoruppa commented 6 years ago

Here's my 2 cents -- I agree that trying to distinguish a package-info.java from normal Java files by the file contents would be unreliable, and, therefore, a bad idea.

The distinguishing characteristic of a package-info.java is, well, its file name. That's all there is to it. So if you do not have the storage information, and thus you do not have a file name, you simply don't know. Period. So the method isPackageInfo() should somehow communicate to its caller that it doesn't know. Therefore:

matozoid commented 6 years ago

The reason I came up with my second paragraph is this part of the JLS: "The following scheme is strongly recommended for file-system-based implementations: ... is placed in a source file called package-info.java"

"Strongly recommended" - thank you JLS, for your 99% certain specifications... :-(

malteskoruppa commented 6 years ago

I understand. :-)

But, because of the missing 1%, it would feel a bit like "guessing". :-) I think that the JavaParser API should be well-defined and reliable.

If a user calls isPackageInfo() and gets a "not sure" answer (because no storage information is available), that user can still decide to apply heuristics themselves, like "Is there only a package declaration in this compilation unit and nothing else?", depending on what they want/need. But it's not something JavaParser should do IMO, because, well, as we all agree, it's unreliable. :-)

Also, not only can a package-info.java contain more than just a package declaration (even though that'd be bad form); conversely, a .java file that is not a package-info.java can theoretically contain nothing else but a package declaration (and therefore "look like" a package-info.java, even though it's not). It wouldn't make a lot of sense, but it'd be legal from a language perspective.