Merge pull request github#5142 from Marcono1234/marcono1234/maven-pom-improvements

Java: Improve MavenPom documentation, rename inconsistent predicates

Author: smowton

java/ql/src/semmle/code/xml/MavenPom.qll

@@ -28,7 +28,7 @@ class ProtoPom extends XMLElement {
   Version getVersion() { result = this.getAChild() }
 
   /**
-   * Gets a string representing the version, or an empty string if no version
+   * Gets a string representing the version, or an empty string if no `version`
    * tag was provided.
    */
   string getVersionString() {
@@ -53,7 +53,7 @@ class Pom extends ProtoPom {
   Pom() {
     this.getName() = "project" and
     // Ignore "dependency-reduced-pom" files - these are generated by the
-    // shading plugin, and duplicate existing pom files.
+    // Maven Shade Plugin, and duplicate existing POM files.
     this.getFile().getStem() != "dependency-reduced-pom"
   }
 
@@ -77,7 +77,7 @@ class Pom extends ProtoPom {
   /** Gets a child XML element named "dependencies". */
   Dependencies getDependencies() { result = this.getAChild() }
 
-  /** Gets a child XML element named `dependencyManagement`. */
+  /** Gets a child XML element named "dependencyManagement". */
   DependencyManagement getDependencyManagement() { result = getAChild() }
 
   /** Gets a Dependency element for this POM. */
@@ -100,7 +100,8 @@ class Pom extends ProtoPom {
   }
 
   /**
-   * Gets a property value defined for this project with the given name.
+   * Gets a property value defined for this project with the given name, either in a local
+   * `<properties>` section, or in the `<properties>` section of an ancestor POM.
    */
   PomProperty getProperty(string name) {
     result.getName() = name and
@@ -112,7 +113,7 @@ class Pom extends ProtoPom {
    */
   PomElement getProjectProperty() {
     (
-      // It must either be a child of the pom, or a child of the parent node of the pom
+      // It must either be a child of the POM, or a child of the parent node of the POM
       result = getAChild()
       or
       result = getParentPom().getAChild() and
@@ -124,8 +125,8 @@ class Pom extends ProtoPom {
   }
 
   /**
-   * Resolve the given placeholder (if possible) in the static context of this pom. Resolution
-   * occurs by considering the properties defined by this project.
+   * Resolve the given placeholder (if possible) in the static context of this POM. Resolution
+   * occurs by considering the properties defined by this project or an ancestor project.
    */
   string resolvePlaceholder(string name) {
     if name.prefix(8) = "project."
@@ -142,32 +143,33 @@ class Pom extends ProtoPom {
   }
 
   /**
-   * Gets all the dependencies that are exported by this pom. An exported dependency is one that
-   * is transitively available, i.e. one with scope compile.
+   * Gets all the dependencies that are exported by this POM. An exported dependency is one that
+   * is transitively available, i.e. one with scope "compile".
    */
   Dependency getAnExportedDependency() {
     result = getADependency() and result.getScope() = "compile"
   }
 
   /**
-   * Gets a pom dependency that is exported by this pom. An exported dependency is one that
-   * is transitively available, i.e. one with scope compile.
+   * Gets a POM dependency that is exported by this POM. An exported dependency is one that
+   * is transitively available, i.e. one with scope "compile".
    */
   Pom getAnExportedPom() { result = getAnExportedDependency().getPom() }
 
   /**
-   * Gets the `<parent>` element of this pom, if any.
+   * Gets the `<parent>` element of this POM, if any.
    */
   Parent getParentElement() { result = getAChild() }
 
   /**
-   * Gets the pom referred to by the `<parent>` element of this pom, if any.
+   * Gets the POM referred to by the `<parent>` element of this POM, if any.
    */
   Pom getParentPom() { result = getParentElement().getPom() }
 
   /**
    * Gets the version specified for dependency `dep` in a `dependencyManagement`
-   * section if this pom or one of its ancestors.
+   * section in this POM or one of its ancestors, or an empty string if no version
+   * is specified.
    */
   string getVersionStringForDependency(Dependency dep) {
     if exists(getDependencyManagement().getDependency(dep))
@@ -223,12 +225,13 @@ class Dependency extends ProtoPom {
   Pom getPom() { result.getShortCoordinate() = this.getShortCoordinate() }
 
   /**
-   * Gets the jar file that we think maven resolved this dependency to (if any).
+   * Gets the jar file that Maven likely resolved this dependency to (if any).
+   * See `MavenRepo.getAnArtifact(ProtoPom)` for how this match is determined.
    */
   File getJar() { exists(MavenRepo mr | result = mr.getAnArtifact(this)) }
 
   /**
-   * Gets the scope of this dependency. If the scope tag is present, this will
+   * Gets the scope of this dependency. If the `scope` tag is present, this will
    * be the string contents of that tag, otherwise it defaults to "compile".
    */
   string getScope() {
@@ -249,14 +252,14 @@ class Dependency extends ProtoPom {
 }
 
 /**
- * A Maven dependency element that represents an actual dependency from a given pom project.
+ * A Maven dependency element that represents an actual dependency from a given POM project.
  */
 class PomDependency extends Dependency {
   PomDependency() {
     exists(Pom source |
-      // This dependency must be a dependency of a pom - dependency tags can also appear in the dependency
-      // management section, where they do not directly contribute to the dependencies of the containing
-      // pom.
+      // This dependency must be a dependency of a POM - dependency tags can also appear in the
+      // dependencyManagement section, where they do not directly contribute to the dependencies of
+      // the containing POM.
       source.getADependency() = this and
       // Consider dependencies that can be used at compile time.
       (
@@ -284,7 +287,7 @@ class PomElement extends XMLElement {
       s = allCharactersString() and
       if s.matches("${%")
       then
-        // Resolve the placeholder in the parent pom
+        // Resolve the placeholder in the parent POM
         result = getParent*().(Pom).resolvePlaceholder(s.substring(2, s.length() - 1))
       else result = s
     )
@@ -330,7 +333,7 @@ class Dependencies extends PomElement {
   Dependency getADependency() { result = this.getAChild() }
 }
 
-/** An XML element named `dependencyManagement`, as found in Maven POM XML files. */
+/** An XML element named "dependencyManagement", as found in Maven POM XML files. */
 class DependencyManagement extends PomElement {
   DependencyManagement() { getName() = "dependencyManagement" }
 
@@ -349,7 +352,7 @@ class DependencyManagement extends PomElement {
 }
 
 /**
- * An XML element name "properties", as found in Maven POM XML files.
+ * An XML element named "properties", as found in Maven POM XML files.
  */
 class PomProperties extends PomElement {
   PomProperties() { this.getName() = "properties" }
@@ -366,8 +369,8 @@ class PomProperty extends PomElement {
 }
 
 /**
- * A folder that represents a maven local repository using the standard layout. Any folder called
- * "repository" with a parent name ".m2" is considered to be a maven repository.
+ * A folder that represents a local Maven repository using the standard layout. Any folder called
+ * "repository" with a parent name ".m2" is considered to be a Maven repository.
  */
 class MavenRepo extends Folder {
   MavenRepo() { getBaseName() = "repository" and getParentContainer().getBaseName() = ".m2" }
@@ -378,18 +381,18 @@ class MavenRepo extends Folder {
   File getAJarFile() { result = getAChildContainer*().(File) and result.getExtension() = "jar" }
 
   /**
-   * Gets any jar artifacts in this repository that match the pom project definition. This is an
-   * over approximation. For soft qualifiers (e.g. 1.0) we return precise matches in preference to
-   * artefact only matches. For hard qualifiers (e.g. [1.0]) we return only precise matches. For
-   * all other qualifiers, we return all matches regardless of version.
+   * Gets any jar artifacts in this repository that match the POM project definition. This is an
+   * over approximation. For soft qualifiers (e.g. 1.0) precise matches are returned in preference
+   * to artifact-only matches. For hard qualifiers (e.g. [1.0]) only precise matches are returned.
+   * For all other qualifiers, all matches are returned regardless of version.
    */
   MavenRepoJar getAnArtifact(ProtoPom pom) {
     result = getAJarFile() and
     if exists(MavenRepoJar mrj | mrj.preciseMatch(pom)) or versionHardMatch(pom)
     then
       // Either a hard match qualifier, or soft and there is at least one precise match
       result.preciseMatch(pom)
-    else result.artefactMatches(pom)
+    else result.artifactMatches(pom)
   }
 }
 
@@ -401,16 +404,19 @@ private predicate versionHardMatch(ProtoPom pom) {
 }
 
 /**
- * A jar file inside a maven repository.
+ * A jar file inside a Maven repository.
  *
  * See: https://cwiki.apache.org/confluence/display/MAVENOLD/Repository+Layout+-+Final
  */
 class MavenRepoJar extends File {
   MavenRepoJar() { exists(MavenRepo mr | mr.getAJarFile() = this) }
 
-  string getGroupID() {
+  /**
+   * Gets the `groupId` of this jar.
+   */
+  string getGroupId() {
     exists(MavenRepo mr | mr.getAJarFile() = this |
-      // Assuming the standard layout, the first part of the directory structure from the maven
+      // Assuming the standard layout, the first part of the directory structure from the Maven
       // repository will be the groupId converted to a path by replacing "." with "/".
       result =
         getParentContainer()
@@ -422,24 +428,30 @@ class MavenRepoJar extends File {
     )
   }
 
-  string getArtefactID() { result = getParentContainer().getParentContainer().getBaseName() }
+  /**
+   * Gets the `artifactId` of this jar.
+   */
+  string getArtifactId() { result = getParentContainer().getParentContainer().getBaseName() }
 
+  /**
+   * Gets the artifact version string of this jar.
+   */
   string getVersion() { result = getParentContainer().getBaseName() }
 
   /**
-   * Holds if this jar is an artefact for the given pom or dependency, regardless of which version it is.
+   * Holds if this jar is an artifact for the given POM or dependency, regardless of which version it is.
    */
-  predicate artefactMatches(ProtoPom pom) {
-    pom.getGroup().getValue() = getGroupID() and
-    pom.getArtifact().getValue() = getArtefactID()
+  predicate artifactMatches(ProtoPom pom) {
+    pom.getGroup().getValue() = getGroupId() and
+    pom.getArtifact().getValue() = getArtifactId()
   }
 
   /**
-   * Holds if this jar is both an artefact for the pom, and has a version string that matches the pom
+   * Holds if this jar is both an artifact for the POM, and has a version string that matches the POM
    * version string. Only soft and hard version matches are supported.
    */
   predicate preciseMatch(ProtoPom pom) {
-    artefactMatches(pom) and
+    artifactMatches(pom) and
     if versionHardMatch(pom)
     then ("[" + getVersion() + "]").matches(pom.getVersionString() + "%")
     else getVersion().matches(pom.getVersionString() + "%")