From 44826ab446d1115d464e73e7e308df36dcf7d39b Mon Sep 17 00:00:00 2001 From: Christian Schulte Date: Mon, 14 Dec 2015 04:57:47 +0100 Subject: [PATCH] [MNG-6164] Collections inconsistently immutable Make non-empty collections returned immutable just like those returned by java.util.Collections. --- .../maven/artifact/DefaultArtifact.java | 2 +- .../artifact/versioning/VersionRange.java | 2 +- .../repository/DefaultArtifactRepository.java | 2 +- .../repository/MetadataResolutionResult.java | 27 +++++++++++++------ .../repository/MavenArtifactRepository.java | 2 +- .../resolver/ArtifactResolutionResult.java | 27 +++++++++++++------ .../artifact/resolver/ResolutionNode.java | 1 + .../maven/exception/ExceptionSummary.java | 5 +++- .../DefaultMavenExecutionResult.java | 6 +++-- .../lifecycle/internal/MojoExecutor.java | 18 +++++++------ .../internal/DefaultMavenPluginManager.java | 2 +- .../prefix/DefaultPluginPrefixRequest.java | 4 +-- .../version/DefaultPluginVersionRequest.java | 2 +- .../DefaultDependencyResolutionResult.java | 5 +++- .../maven/project/DefaultProjectBuilder.java | 1 + .../project/DefaultProjectRealmCache.java | 4 ++- .../apache/maven/project/MavenProject.java | 19 +++++++------ .../project/artifact/ProjectArtifact.java | 7 +++-- .../java/org/apache/maven/cli/MavenCli.java | 4 ++- .../building/ModelBuildingException.java | 2 +- 20 files changed, 91 insertions(+), 51 deletions(-) diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/DefaultArtifact.java b/maven-artifact/src/main/java/org/apache/maven/artifact/DefaultArtifact.java index 1167e91a53..3fa1907c31 100644 --- a/maven-artifact/src/main/java/org/apache/maven/artifact/DefaultArtifact.java +++ b/maven-artifact/src/main/java/org/apache/maven/artifact/DefaultArtifact.java @@ -273,7 +273,7 @@ public class DefaultArtifact return Collections.emptyList(); } - return metadataMap.values(); + return Collections.unmodifiableCollection( metadataMap.values() ); } // ---------------------------------------------------------------------- diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/VersionRange.java b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/VersionRange.java index 56343b2f6d..e9196213fb 100644 --- a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/VersionRange.java +++ b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/VersionRange.java @@ -262,7 +262,7 @@ public class VersionRange } else { - restrictions = intersection( r1, r2 ); + restrictions = Collections.unmodifiableList( intersection( r1, r2 ) ); } ArtifactVersion version = null; diff --git a/maven-compat/src/main/java/org/apache/maven/artifact/repository/DefaultArtifactRepository.java b/maven-compat/src/main/java/org/apache/maven/artifact/repository/DefaultArtifactRepository.java index f5db5ef634..16b82c5070 100644 --- a/maven-compat/src/main/java/org/apache/maven/artifact/repository/DefaultArtifactRepository.java +++ b/maven-compat/src/main/java/org/apache/maven/artifact/repository/DefaultArtifactRepository.java @@ -256,7 +256,7 @@ public class DefaultArtifactRepository { if ( mirroredRepositories != null ) { - this.mirroredRepositories = mirroredRepositories; + this.mirroredRepositories = Collections.unmodifiableList( mirroredRepositories ); } else { diff --git a/maven-compat/src/main/java/org/apache/maven/repository/MetadataResolutionResult.java b/maven-compat/src/main/java/org/apache/maven/repository/MetadataResolutionResult.java index 70b9c3fdc5..7be1d96950 100644 --- a/maven-compat/src/main/java/org/apache/maven/repository/MetadataResolutionResult.java +++ b/maven-compat/src/main/java/org/apache/maven/repository/MetadataResolutionResult.java @@ -118,7 +118,10 @@ public class MetadataResolutionResult public List getMissingArtifacts() { - return missingArtifacts == null ? Collections.emptyList() : missingArtifacts; + return missingArtifacts == null + ? Collections.emptyList() + : Collections.unmodifiableList( missingArtifacts ); + } public MetadataResolutionResult addMissingArtifact( Artifact artifact ) @@ -148,7 +151,10 @@ public class MetadataResolutionResult public List getExceptions() { - return exceptions == null ? Collections.emptyList() : exceptions; + return exceptions == null + ? Collections.emptyList() + : Collections.unmodifiableList( exceptions ); + } // ------------------------------------------------------------------------ @@ -185,7 +191,10 @@ public class MetadataResolutionResult public List getVersionRangeViolations() { - return versionRangeViolations == null ? Collections.emptyList() : versionRangeViolations; + return versionRangeViolations == null + ? Collections.emptyList() + : Collections.unmodifiableList( versionRangeViolations ); + } // ------------------------------------------------------------------------ @@ -217,8 +226,10 @@ public class MetadataResolutionResult public List getMetadataResolutionExceptions() { - return metadataResolutionExceptions == null ? Collections.emptyList() - : metadataResolutionExceptions; + return metadataResolutionExceptions == null + ? Collections.emptyList() + : Collections.unmodifiableList( metadataResolutionExceptions ); + } // ------------------------------------------------------------------------ @@ -246,7 +257,7 @@ public class MetadataResolutionResult return Collections.emptyList(); } - return errorArtifactExceptions; + return Collections.unmodifiableList( errorArtifactExceptions ); } // ------------------------------------------------------------------------ @@ -283,7 +294,7 @@ public class MetadataResolutionResult return Collections.emptyList(); } - return circularDependencyExceptions; + return Collections.unmodifiableList( circularDependencyExceptions ); } // ------------------------------------------------------------------------ @@ -297,7 +308,7 @@ public class MetadataResolutionResult return Collections.emptyList(); } - return repositories; + return Collections.unmodifiableList( repositories ); } public MetadataResolutionResult setRepositories( final List repositories ) diff --git a/maven-core/src/main/java/org/apache/maven/artifact/repository/MavenArtifactRepository.java b/maven-core/src/main/java/org/apache/maven/artifact/repository/MavenArtifactRepository.java index d4c64f0bf7..f04007ee6e 100644 --- a/maven-core/src/main/java/org/apache/maven/artifact/repository/MavenArtifactRepository.java +++ b/maven-core/src/main/java/org/apache/maven/artifact/repository/MavenArtifactRepository.java @@ -405,7 +405,7 @@ public class MavenArtifactRepository { if ( mirroredRepositories != null ) { - this.mirroredRepositories = mirroredRepositories; + this.mirroredRepositories = Collections.unmodifiableList( mirroredRepositories ); } else { diff --git a/maven-core/src/main/java/org/apache/maven/artifact/resolver/ArtifactResolutionResult.java b/maven-core/src/main/java/org/apache/maven/artifact/resolver/ArtifactResolutionResult.java index 6de04f3cbd..ffae1d3560 100644 --- a/maven-core/src/main/java/org/apache/maven/artifact/resolver/ArtifactResolutionResult.java +++ b/maven-core/src/main/java/org/apache/maven/artifact/resolver/ArtifactResolutionResult.java @@ -130,7 +130,10 @@ public class ArtifactResolutionResult public List getMissingArtifacts() { - return missingArtifacts == null ? Collections.emptyList() : missingArtifacts; + return missingArtifacts == null + ? Collections.emptyList() + : Collections.unmodifiableList( missingArtifacts ); + } public ArtifactResolutionResult addMissingArtifact( Artifact artifact ) @@ -165,7 +168,10 @@ public class ArtifactResolutionResult public List getExceptions() { - return exceptions == null ? Collections.emptyList() : exceptions; + return exceptions == null + ? Collections.emptyList() + : Collections.unmodifiableList( exceptions ); + } // ------------------------------------------------------------------------ @@ -202,7 +208,10 @@ public class ArtifactResolutionResult public List getVersionRangeViolations() { - return versionRangeViolations == null ? Collections.emptyList() : versionRangeViolations; + return versionRangeViolations == null + ? Collections.emptyList() + : Collections.unmodifiableList( versionRangeViolations ); + } // ------------------------------------------------------------------------ @@ -234,8 +243,10 @@ public class ArtifactResolutionResult public List getMetadataResolutionExceptions() { - return metadataResolutionExceptions == null ? Collections.emptyList() - : metadataResolutionExceptions; + return metadataResolutionExceptions == null + ? Collections.emptyList() + : Collections.unmodifiableList( metadataResolutionExceptions ); + } // ------------------------------------------------------------------------ @@ -267,7 +278,7 @@ public class ArtifactResolutionResult return Collections.emptyList(); } - return errorArtifactExceptions; + return Collections.unmodifiableList( errorArtifactExceptions ); } // ------------------------------------------------------------------------ @@ -304,7 +315,7 @@ public class ArtifactResolutionResult return Collections.emptyList(); } - return circularDependencyExceptions; + return Collections.unmodifiableList( circularDependencyExceptions ); } // ------------------------------------------------------------------------ @@ -318,7 +329,7 @@ public class ArtifactResolutionResult return Collections.emptyList(); } - return repositories; + return Collections.unmodifiableList( repositories ); } public ArtifactResolutionResult setRepositories( final List repositories ) diff --git a/maven-core/src/main/java/org/apache/maven/artifact/resolver/ResolutionNode.java b/maven-core/src/main/java/org/apache/maven/artifact/resolver/ResolutionNode.java index a156871de0..8e47d2e0b4 100644 --- a/maven-core/src/main/java/org/apache/maven/artifact/resolver/ResolutionNode.java +++ b/maven-core/src/main/java/org/apache/maven/artifact/resolver/ResolutionNode.java @@ -102,6 +102,7 @@ public class ResolutionNode children.add( new ResolutionNode( a, remoteRepositories, this ) ); } + children = Collections.unmodifiableList( children ); } else { diff --git a/maven-core/src/main/java/org/apache/maven/exception/ExceptionSummary.java b/maven-core/src/main/java/org/apache/maven/exception/ExceptionSummary.java index dcc376a4b9..6615af61ce 100644 --- a/maven-core/src/main/java/org/apache/maven/exception/ExceptionSummary.java +++ b/maven-core/src/main/java/org/apache/maven/exception/ExceptionSummary.java @@ -53,7 +53,10 @@ public class ExceptionSummary this.exception = exception; this.message = ( message != null ) ? message : ""; this.reference = ( reference != null ) ? reference : ""; - this.children = ( children != null ) ? children : Collections.emptyList(); + this.children = ( children != null ) + ? Collections.unmodifiableList( children ) + : Collections.emptyList(); + } public Throwable getException() diff --git a/maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionResult.java b/maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionResult.java index 87d8676627..112a064923 100644 --- a/maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionResult.java +++ b/maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionResult.java @@ -64,8 +64,10 @@ public class DefaultMavenExecutionResult public List getTopologicallySortedProjects() { - return null == topologicallySortedProjects ? Collections.emptyList() - : topologicallySortedProjects; + return null == topologicallySortedProjects + ? Collections.emptyList() + : Collections.unmodifiableList( topologicallySortedProjects ); + } public DependencyResolutionResult getDependencyResolutionResult() diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java index 766aed1469..b78f54dc42 100644 --- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java @@ -105,32 +105,34 @@ public class MojoExecutor private Collection toScopes( String classpath ) { + Collection scopes = Collections.emptyList(); + if ( StringUtils.isNotEmpty( classpath ) ) { if ( Artifact.SCOPE_COMPILE.equals( classpath ) ) { - return Arrays.asList( Artifact.SCOPE_COMPILE, Artifact.SCOPE_SYSTEM, Artifact.SCOPE_PROVIDED ); + scopes = Arrays.asList( Artifact.SCOPE_COMPILE, Artifact.SCOPE_SYSTEM, Artifact.SCOPE_PROVIDED ); } else if ( Artifact.SCOPE_RUNTIME.equals( classpath ) ) { - return Arrays.asList( Artifact.SCOPE_COMPILE, Artifact.SCOPE_RUNTIME ); + scopes = Arrays.asList( Artifact.SCOPE_COMPILE, Artifact.SCOPE_RUNTIME ); } else if ( Artifact.SCOPE_COMPILE_PLUS_RUNTIME.equals( classpath ) ) { - return Arrays.asList( Artifact.SCOPE_COMPILE, Artifact.SCOPE_SYSTEM, Artifact.SCOPE_PROVIDED, - Artifact.SCOPE_RUNTIME ); + scopes = Arrays.asList( Artifact.SCOPE_COMPILE, Artifact.SCOPE_SYSTEM, Artifact.SCOPE_PROVIDED, + Artifact.SCOPE_RUNTIME ); } else if ( Artifact.SCOPE_RUNTIME_PLUS_SYSTEM.equals( classpath ) ) { - return Arrays.asList( Artifact.SCOPE_COMPILE, Artifact.SCOPE_SYSTEM, Artifact.SCOPE_RUNTIME ); + scopes = Arrays.asList( Artifact.SCOPE_COMPILE, Artifact.SCOPE_SYSTEM, Artifact.SCOPE_RUNTIME ); } else if ( Artifact.SCOPE_TEST.equals( classpath ) ) { - return Arrays.asList( Artifact.SCOPE_COMPILE, Artifact.SCOPE_SYSTEM, Artifact.SCOPE_PROVIDED, - Artifact.SCOPE_RUNTIME, Artifact.SCOPE_TEST ); + scopes = Arrays.asList( Artifact.SCOPE_COMPILE, Artifact.SCOPE_SYSTEM, Artifact.SCOPE_PROVIDED, + Artifact.SCOPE_RUNTIME, Artifact.SCOPE_TEST ); } } - return Collections.emptyList(); + return Collections.unmodifiableCollection( scopes ); } public void execute( MavenSession session, List mojoExecutions, ProjectIndex projectIndex ) diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java index cec6f120db..86304714df 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java @@ -461,7 +461,7 @@ public class DefaultMavenPluginManager it.remove(); } } - return artifacts; + return Collections.unmodifiableList( artifacts ); } private Map calcImports( MavenProject project, ClassLoader parent, List imports ) diff --git a/maven-core/src/main/java/org/apache/maven/plugin/prefix/DefaultPluginPrefixRequest.java b/maven-core/src/main/java/org/apache/maven/plugin/prefix/DefaultPluginPrefixRequest.java index 7ab86cfd16..01194c866d 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/prefix/DefaultPluginPrefixRequest.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/prefix/DefaultPluginPrefixRequest.java @@ -100,7 +100,7 @@ public class DefaultPluginPrefixRequest { if ( pluginGroups != null ) { - this.pluginGroups = pluginGroups; + this.pluginGroups = Collections.unmodifiableList( pluginGroups ); } else { @@ -131,7 +131,7 @@ public class DefaultPluginPrefixRequest { if ( repositories != null ) { - this.repositories = repositories; + this.repositories = Collections.unmodifiableList( repositories ); } else { diff --git a/maven-core/src/main/java/org/apache/maven/plugin/version/DefaultPluginVersionRequest.java b/maven-core/src/main/java/org/apache/maven/plugin/version/DefaultPluginVersionRequest.java index 9907066b9d..57f4250c63 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/version/DefaultPluginVersionRequest.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/version/DefaultPluginVersionRequest.java @@ -140,7 +140,7 @@ public class DefaultPluginVersionRequest { if ( repositories != null ) { - this.repositories = repositories; + this.repositories = Collections.unmodifiableList( repositories ); } else { diff --git a/maven-core/src/main/java/org/apache/maven/project/DefaultDependencyResolutionResult.java b/maven-core/src/main/java/org/apache/maven/project/DefaultDependencyResolutionResult.java index dbdf25261d..509e8b48d9 100644 --- a/maven-core/src/main/java/org/apache/maven/project/DefaultDependencyResolutionResult.java +++ b/maven-core/src/main/java/org/apache/maven/project/DefaultDependencyResolutionResult.java @@ -98,7 +98,10 @@ class DefaultDependencyResolutionResult public List getResolutionErrors( Dependency dependency ) { List errors = resolutionErrors.get( dependency ); - return ( errors != null ) ? errors : Collections.emptyList(); + return ( errors != null ) + ? Collections.unmodifiableList( errors ) + : Collections.emptyList(); + } public void setResolutionErrors( Dependency dependency, List errors ) diff --git a/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java b/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java index 1091a084c9..a6590463a0 100644 --- a/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java +++ b/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java @@ -809,6 +809,7 @@ public class DefaultProjectBuilder map.put( d.getManagementKey(), artifact ); } } + map = Collections.unmodifiableMap( map ); } else { diff --git a/maven-core/src/main/java/org/apache/maven/project/DefaultProjectRealmCache.java b/maven-core/src/main/java/org/apache/maven/project/DefaultProjectRealmCache.java index 987f861a18..a7f06156a3 100644 --- a/maven-core/src/main/java/org/apache/maven/project/DefaultProjectRealmCache.java +++ b/maven-core/src/main/java/org/apache/maven/project/DefaultProjectRealmCache.java @@ -51,7 +51,9 @@ public class DefaultProjectRealmCache public CacheKey( List extensionRealms ) { - this.extensionRealms = ( extensionRealms != null ) ? extensionRealms : Collections.emptyList(); + this.extensionRealms = ( extensionRealms != null ) + ? Collections.unmodifiableList( extensionRealms ) + : Collections.emptyList(); this.hashCode = this.extensionRealms.hashCode(); } diff --git a/maven-core/src/main/java/org/apache/maven/project/MavenProject.java b/maven-core/src/main/java/org/apache/maven/project/MavenProject.java index dff5f3fdcf..fd7ab40ddc 100644 --- a/maven-core/src/main/java/org/apache/maven/project/MavenProject.java +++ b/maven-core/src/main/java/org/apache/maven/project/MavenProject.java @@ -778,7 +778,7 @@ public class MavenProject { return Collections.emptyList(); } - return getModel().getBuild().getPlugins(); + return Collections.unmodifiableList( getModel().getBuild().getPlugins() ); } public List getModules() @@ -1079,7 +1079,7 @@ public class MavenProject } else { - return build.getExtensions(); + return Collections.unmodifiableList( build.getExtensions() ); } } @@ -1604,7 +1604,7 @@ public class MavenProject { // TODO let the scope handler deal with this if ( Artifact.SCOPE_COMPILE.equals( a.getScope() ) || Artifact.SCOPE_PROVIDED.equals( a.getScope() ) - || Artifact.SCOPE_SYSTEM.equals( a.getScope() ) ) + || Artifact.SCOPE_SYSTEM.equals( a.getScope() ) ) { Dependency dependency = new Dependency(); @@ -1618,7 +1618,7 @@ public class MavenProject list.add( dependency ); } } - return list; + return Collections.unmodifiableList( list ); } @Deprecated @@ -1662,7 +1662,7 @@ public class MavenProject list.add( dependency ); } - return list; + return Collections.unmodifiableList( list ); } @Deprecated // used by the Maven ITs @@ -1677,7 +1677,7 @@ public class MavenProject List list = new ArrayList<>( artifacts.size() ); - for ( Artifact a : getArtifacts() ) + for ( Artifact a : getArtifacts() ) { // TODO let the scope handler deal with this if ( Artifact.SCOPE_COMPILE.equals( a.getScope() ) || Artifact.SCOPE_RUNTIME.equals( a.getScope() ) ) @@ -1694,7 +1694,7 @@ public class MavenProject list.add( dependency ); } } - return list; + return Collections.unmodifiableList( list ); } @Deprecated @@ -1790,7 +1790,7 @@ public class MavenProject list.add( dependency ); } } - return list; + return Collections.unmodifiableList( list ); } @Deprecated @@ -1862,8 +1862,7 @@ public class MavenProject { return Collections.emptyList(); } - return getModel().getReporting().getPlugins(); - + return Collections.unmodifiableList( getModel().getReporting().getPlugins() ); } @Deprecated diff --git a/maven-core/src/main/java/org/apache/maven/project/artifact/ProjectArtifact.java b/maven-core/src/main/java/org/apache/maven/project/artifact/ProjectArtifact.java index 8556a6af64..46e56dc470 100644 --- a/maven-core/src/main/java/org/apache/maven/project/artifact/ProjectArtifact.java +++ b/maven-core/src/main/java/org/apache/maven/project/artifact/ProjectArtifact.java @@ -58,8 +58,11 @@ public class ProjectArtifact public List getManagedDependencies() { - DependencyManagement depMgmt = project.getDependencyManagement(); - return ( depMgmt != null ) ? depMgmt.getDependencies() : Collections.emptyList(); + DependencyManagement depMngt = project.getDependencyManagement(); + return ( depMngt != null ) + ? Collections.unmodifiableList( depMngt.getDependencies() ) + : Collections.emptyList(); + } static class PomArtifactHandler diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java index ca8e040732..6a1a34ad55 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java @@ -750,7 +750,9 @@ public class MavenCli BootstrapCoreExtensionManager resolver = container.lookup( BootstrapCoreExtensionManager.class ); - return resolver.loadCoreExtensions( request, providedArtifacts, extensions ); + return Collections.unmodifiableList( resolver.loadCoreExtensions( request, providedArtifacts, + extensions ) ); + } finally { diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingException.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingException.java index 434cb591a2..b5274382be 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingException.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingException.java @@ -136,7 +136,7 @@ public class ModelBuildingException { return Collections.emptyList(); } - return result.getProblems(); + return Collections.unmodifiableList( result.getProblems() ); } private static String toMessage( ModelBuildingResult result )