[MNG-4383] Uninterpolated expressions should cause an error

git-svn-id: https://svn.apache.org/repos/asf/maven/maven-3/trunk@821772 13f79535-47bb-0310-9956-ffa450edef68

maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java

@@ -139,8 +139,7 @@ public class DefaultModelValidator
 
         validateStringNotEmpty( "version", problems, false, model.getVersion() );
 
-        boolean warnOnBadBoolean = request.getValidationLevel() < ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_0;
-        boolean warnOnBadDependencyScope = request.getValidationLevel() < ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_0;
+        boolean warnOnly = request.getValidationLevel() < ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_0;
 
         for ( Dependency d : model.getDependencies() )
         {
@@ -178,12 +177,15 @@ public class DefaultModelValidator
 
             if ( request.getValidationLevel() >= ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_2_0 )
             {
-                validateBoolean( "dependencies.dependency.optional", problems, warnOnBadBoolean, d.getOptional(),
+                validateVersion( "dependencies.dependency.version", problems, warnOnly, d.getVersion(),
+                                 d.getManagementKey() );
+
+                validateBoolean( "dependencies.dependency.optional", problems, warnOnly, d.getOptional(),
                                  d.getManagementKey() );
 
                 /*
                  * TODO: Extensions like Flex Mojos use custom scopes like "merged", "internal", "external", etc. In
-                 * order to don't break backward-compat with those, only warn but don't error our.
+                 * order to don't break backward-compat with those, only warn but don't error out.
                  */
                 validateEnum( "dependencies.dependency.scope", problems, true, d.getScope(),
                               d.getManagementKey(), "provided", "compile", "runtime", "test", "system" );
@@ -227,8 +229,8 @@ public class DefaultModelValidator
 
                 if ( request.getValidationLevel() >= ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_2_0 )
                 {
-                    validateBoolean( "dependencyManagement.dependencies.dependency.optional", problems,
-                                     warnOnBadBoolean, d.getOptional(), d.getManagementKey() );
+                    validateBoolean( "dependencyManagement.dependencies.dependency.optional", problems, warnOnly,
+                                     d.getOptional(), d.getManagementKey() );
                 }
             }
         }
@@ -250,16 +252,16 @@ public class DefaultModelValidator
                     validateStringNotEmpty( "build.plugins.plugin.version", problems, warnOnMissingPluginVersion,
                                             p.getVersion(), p.getKey() );
 
-                    validateBoolean( "build.plugins.plugin.inherited", problems, warnOnBadBoolean, p.getInherited(),
+                    validateBoolean( "build.plugins.plugin.inherited", problems, warnOnly, p.getInherited(),
                                      p.getKey() );
 
-                    validateBoolean( "build.plugins.plugin.extensions", problems, warnOnBadBoolean, p.getExtensions(),
+                    validateBoolean( "build.plugins.plugin.extensions", problems, warnOnly, p.getExtensions(),
                                      p.getKey() );
 
                     for ( Dependency d : p.getDependencies() )
                     {
                         validateEnum( "build.plugins.plugin[" + p.getKey() + "].dependencies.dependency.scope",
-                                      problems, warnOnBadDependencyScope, d.getScope(), d.getManagementKey(),
+                                      problems, warnOnly, d.getScope(), d.getManagementKey(),
                                       "compile", "runtime", "system" );
                     }
                 }
@@ -578,11 +580,12 @@ public class DefaultModelValidator
 
         if ( sourceHint != null )
         {
-            addViolation( problems, warning, "'" + fieldName + "' must be 'true' or 'false' for " + sourceHint );
+            addViolation( problems, warning, "'" + fieldName + "' must be 'true' or 'false' for " + sourceHint
+                + " but is '" + string + "'." );
         }
         else
         {
-            addViolation( problems, warning, "'" + fieldName + "' must be 'true' or 'false'." );
+            addViolation( problems, warning, "'" + fieldName + "' must be 'true' or 'false' but is '" + string + "'." );
         }
 
         return false;
@@ -605,11 +608,39 @@ public class DefaultModelValidator
 
         if ( sourceHint != null )
         {
-            addViolation( problems, warning, "'" + fieldName + "' must be one of " + values + " for " + sourceHint );
+            addViolation( problems, warning, "'" + fieldName + "' must be one of " + values + " for " + sourceHint
+                + " but is '" + string + "'." );
         }
         else
         {
-            addViolation( problems, warning, "'" + fieldName + "' must be one of " + values );
+            addViolation( problems, warning, "'" + fieldName + "' must be one of " + values + " but is '" + string
+                + "'." );
+        }
+
+        return false;
+    }
+
+    private boolean validateVersion( String fieldName, ModelProblemCollector problems, boolean warning, String string,
+                                     String sourceHint )
+    {
+        if ( string == null || string.length() <= 0 )
+        {
+            return true;
+        }
+
+        if ( !hasExpression( string ) )
+        {
+            return true;
+        }
+
+        if ( sourceHint != null )
+        {
+            addViolation( problems, warning, "'" + fieldName + "' must be a valid version for " + sourceHint
+                + " but is '" + string + "'." );
+        }
+        else
+        {
+            addViolation( problems, warning, "'" + fieldName + "' must be a valid version but is '" + string + "'." );
         }
 
         return false;

maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java

@@ -334,4 +334,14 @@ public class DefaultModelValidatorTest
         assertTrue( result.getWarnings().get( 1 ).contains( "test:g" ) );
     }
 
+    public void testBadDependencyVersion()
+        throws Exception
+    {
+        SimpleProblemCollector result = validate( "bad-dependency-version.xml" );
+
+        assertViolations( result, 1, 0 );
+
+        assertTrue( result.getErrors().get( 0 ).contains( "test:b" ) );
+    }
+
 }

maven-model-builder/src/test/resources/poms/validation/bad-dependency-version.xml

@@ -0,0 +1,38 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+<project>
+  <modelVersion>4.0.0</modelVersion>
+  <artifactId>aid</artifactId>
+  <groupId>gid</groupId>
+  <version>0.1</version>
+
+  <dependencies>
+    <dependency>
+      <groupId>test</groupId>
+      <artifactId>a</artifactId>
+      <version>0.2</version>
+    </dependency>
+    <dependency>
+      <groupId>test</groupId>
+      <artifactId>b</artifactId>
+      <version>${missing.property}</version>
+    </dependency>
+  </dependencies>
+</project>