Merge pull request #421 from Johnlon/master

Implemented required includes -- again

HOCON.md

@@ -906,6 +906,7 @@ followed by whitespace and then either:
  - `url()`, `file()`, or `classpath()` surrounding a quoted string
    which is then interpreted as a URL, file, or classpath. The
    string must be quoted, unlike in CSS.
+ - `required()` surrounding one of the above
 
 An include statement can appear in place of an object field.
 
@@ -1031,12 +1032,24 @@ get a value from a system property or from the reference
 configuration. So it's not enough to only look up the "fixed up"
 path, it's necessary to look up the original path as well.
 
-#### Include semantics: missing files
+#### Include semantics: missing files and required files
 
-If an included file does not exist, the include statement should
+By default, if an included file does not exist then the include statement should
 be silently ignored (as if the included file contained only an
 empty object).
 
+If however an included resource is mandatory then the name of the
+included resource may be wrapped with `required()`, in which case
+file parsing will fail with an error if the resource cannot be resolved.
+
+The syntax for this is
+
+    include required("foo.conf")
+    include required(file("foo.conf"))
+    include required(classpath("foo.conf"))
+    include required(url("http://localhost/foo.conf"))
+
+
 Other IO errors probably should not be ignored but implementations
 will have to make a judgment which IO errors reflect an ignorable
 missing file, and which reflect a problem to bring to the user's
@@ -1570,7 +1583,7 @@ The only way to ensure that your environment variables have the desired case
 is to first undefine all the env vars that you will depend on then redefine
 them with the required case.
 
-For example, the the ambient environment might have this defition ...
+For example, the the ambient environment might have this definition ...
 
 ```
 set Path=A;B;C

README.md

@@ -869,3 +869,4 @@ format.
 #### Linting tool
 
    * A web based linting tool http://www.hoconlint.com/
+

config/src/main/java/com/typesafe/config/ConfigIncludeContext.java

@@ -43,4 +43,14 @@ public interface ConfigIncludeContext {
      * @return the parse options
      */
     ConfigParseOptions parseOptions();
+
+
+    /**
+     * Copy this {@link ConfigIncludeContext} giving it a new value for its parseOptions.
+     *
+     * @param options new parse options to use
+     *
+     * @return the updated copy of this context
+     */
+    ConfigIncludeContext setParseOptions(ConfigParseOptions options);
 }

config/src/main/java/com/typesafe/config/ConfigParseOptions.java

@@ -111,7 +111,8 @@ public final class ConfigParseOptions {
     /**
      * Set to false to throw an exception if the item being parsed (for example
      * a file) is missing. Set to true to just return an empty document in that
-     * case.
+     * case. Note that this setting applies on only to fetching the root document,
+     * it has no effect on any nested includes.
      *
      * @param allowMissing true to silently ignore missing item
      * @return options with the "allow missing" flag set

config/src/main/java/com/typesafe/config/impl/ConfigDocumentParser.java

@@ -275,8 +275,7 @@ final class ConfigDocumentParser {
                 }
 
                 if (expression.isEmpty()) {
-                    throw parseError("expecting a close brace or a field name here, got "
-                            + t);
+                    throw parseError(ExpectingClosingParenthesisError + t);
                 }
 
                 putBack(t); // put back the token we ended with
@@ -311,7 +310,48 @@ final class ConfigDocumentParser {
             }
         }
 
+        private final String ExpectingClosingParenthesisError = "expecting a close parentheses ')' here, not: ";
+
         private ConfigNodeInclude parseInclude(ArrayList<AbstractConfigNode> children) {
+
+            Token t = nextTokenCollectingWhitespace(children);
+
+            // we either have a 'required()' or a quoted string or the "file()" syntax
+            if (Tokens.isUnquotedText(t)) {
+                String kindText = Tokens.getUnquotedText(t);
+
+                if (kindText.startsWith("required(")) {
+                    String r = kindText.replaceFirst("required\\(","");
+                    if (r.length()>0) {
+                        putBack(Tokens.newUnquotedText(t.origin(),r));
+                    }
+
+                    children.add(new ConfigNodeSingleToken(t));
+                    //children.add(new ConfigNodeSingleToken(tOpen));
+
+                    ConfigNodeInclude res = parseIncludeResource(children, true);
+
+                    t = nextTokenCollectingWhitespace(children);
+
+                    if (Tokens.isUnquotedText(t) && Tokens.getUnquotedText(t).equals(")")) {
+                        // OK, close paren
+                    } else {
+                        throw parseError(ExpectingClosingParenthesisError + t);
+                    }
+
+                    return res;
+                } else {
+                    putBack(t);
+                    return parseIncludeResource(children, false);
+                }
+            }
+            else {
+                putBack(t);
+                return parseIncludeResource(children, false);
+            }
+        }
+
+        private ConfigNodeInclude parseIncludeResource(ArrayList<AbstractConfigNode> children, boolean isRequired) {
             Token t = nextTokenCollectingWhitespace(children);
 
             // we either have a quoted string or the "file()" syntax
@@ -319,17 +359,25 @@ final class ConfigDocumentParser {
                 // get foo(
                 String kindText = Tokens.getUnquotedText(t);
                 ConfigIncludeKind kind;
+                String prefix;
 
-                if (kindText.equals("url(")) {
+                if (kindText.startsWith("url(")) {
                     kind = ConfigIncludeKind.URL;
-                } else if (kindText.equals("file(")) {
+                    prefix = "url(";
+                } else if (kindText.startsWith("file(")) {
                     kind = ConfigIncludeKind.FILE;
-                } else if (kindText.equals("classpath(")) {
+                    prefix = "file(";
+                } else if (kindText.startsWith("classpath(")) {
                     kind = ConfigIncludeKind.CLASSPATH;
+                    prefix = "classpath(";
                 } else {
                     throw parseError("expecting include parameter to be quoted filename, file(), classpath(), or url(). No spaces are allowed before the open paren. Not expecting: "
                             + t);
                 }
+                String r = kindText.replaceFirst("[^(]*\\(","");
+                if (r.length()>0) {
+                    putBack(Tokens.newUnquotedText(t.origin(),r));
+                }
 
                 children.add(new ConfigNodeSingleToken(t));
 
@@ -338,23 +386,26 @@ final class ConfigDocumentParser {
 
                 // quoted string
                 if (!Tokens.isValueWithType(t, ConfigValueType.STRING)) {
-                    throw parseError("expecting a quoted string inside file(), classpath(), or url(), rather than: "
-                            + t);
+                    throw parseError("expecting include " + prefix + ") parameter to be a quoted string, rather than: " + t);
                 }
                 children.add(new ConfigNodeSimpleValue(t));
                 // skip space after string, inside parens
                 t = nextTokenCollectingWhitespace(children);
 
-                if (Tokens.isUnquotedText(t) && Tokens.getUnquotedText(t).equals(")")) {
+                if (Tokens.isUnquotedText(t) && Tokens.getUnquotedText(t).startsWith(")")) {
+                    String rest = Tokens.getUnquotedText(t).substring(1);
+                    if (rest.length()>0) {
+                        putBack(Tokens.newUnquotedText(t.origin(),rest));
+                    }
                     // OK, close paren
                 } else {
-                    throw parseError("expecting a close parentheses ')' here, not: " + t);
+                    throw parseError(ExpectingClosingParenthesisError + t);
                 }
-                return new ConfigNodeInclude(children, kind);
 
+                return new ConfigNodeInclude(children, kind, isRequired);
             } else if (Tokens.isValueWithType(t, ConfigValueType.STRING)) {
                 children.add(new ConfigNodeSimpleValue(t));
-                return new ConfigNodeInclude(children, ConfigIncludeKind.HEURISTIC);
+                return new ConfigNodeInclude(children, ConfigIncludeKind.HEURISTIC, isRequired);
             } else {
                 throw parseError("include keyword is not followed by a quoted string, but by: " + t);
             }

config/src/main/java/com/typesafe/config/impl/ConfigNodeInclude.java

@@ -6,10 +6,12 @@ import java.util.Collection;
 final class ConfigNodeInclude extends AbstractConfigNode {
     final private ArrayList<AbstractConfigNode> children;
     final private ConfigIncludeKind kind;
+    final private boolean isRequired;
 
-    ConfigNodeInclude(Collection<AbstractConfigNode> children, ConfigIncludeKind kind) {
+    ConfigNodeInclude(Collection<AbstractConfigNode> children, ConfigIncludeKind kind, boolean isRequired) {
         this.children = new ArrayList<AbstractConfigNode>(children);
         this.kind = kind;
+        this.isRequired = isRequired;
     }
 
     final public Collection<AbstractConfigNode> children() {
@@ -29,6 +31,10 @@ final class ConfigNodeInclude extends AbstractConfigNode {
         return kind;
     }
 
+    protected boolean isRequired() {
+        return isRequired;
+    }
+
     protected String name() {
         for (AbstractConfigNode n : children) {
             if (n instanceof ConfigNodeSimpleValue) {

config/src/main/java/com/typesafe/config/impl/ConfigParser.java

@@ -157,6 +157,9 @@ final class ConfigParser {
         }
 
         private void parseInclude(Map<String, AbstractConfigValue> values, ConfigNodeInclude n) {
+            boolean isRequired = n.isRequired();
+            ConfigIncludeContext cic = includeContext.setParseOptions(includeContext.parseOptions().setAllowMissing(!isRequired));
+
             AbstractConfigObject obj;
             switch (n.kind()) {
                 case URL:
@@ -166,21 +169,21 @@ final class ConfigParser {
                     } catch (MalformedURLException e) {
                         throw parseError("include url() specifies an invalid URL: " + n.name(), e);
                     }
-                    obj = (AbstractConfigObject) includer.includeURL(includeContext, url);
+                    obj = (AbstractConfigObject) includer.includeURL(cic, url);
                     break;
 
                 case FILE:
-                    obj = (AbstractConfigObject) includer.includeFile(includeContext,
+                    obj = (AbstractConfigObject) includer.includeFile(cic,
                             new File(n.name()));
                     break;
 
                 case CLASSPATH:
-                    obj = (AbstractConfigObject) includer.includeResources(includeContext, n.name());
+                    obj = (AbstractConfigObject) includer.includeResources(cic, n.name());
                     break;
 
                 case HEURISTIC:
                     obj = (AbstractConfigObject) includer
-                            .include(includeContext, n.name());
+                            .include(cic, n.name());
                     break;
 
                 default:

config/src/main/java/com/typesafe/config/impl/SimpleIncludeContext.java

@@ -10,9 +10,16 @@ import com.typesafe.config.ConfigParseable;
 class SimpleIncludeContext implements ConfigIncludeContext {
 
     private final Parseable parseable;
+    private final ConfigParseOptions options;
 
     SimpleIncludeContext(Parseable parseable) {
         this.parseable = parseable;
+        this.options = SimpleIncluder.clearForInclude(parseable.options());
+    }
+
+    private SimpleIncludeContext(Parseable parseable, ConfigParseOptions options) {
+        this.parseable = parseable;
+        this.options = options;
     }
 
     SimpleIncludeContext withParseable(Parseable parseable) {
@@ -34,6 +41,11 @@ class SimpleIncludeContext implements ConfigIncludeContext {
 
     @Override
     public ConfigParseOptions parseOptions() {
-        return SimpleIncluder.clearForInclude(parseable.options());
+        return options;
+    }
+
+    @Override
+    public ConfigIncludeContext setParseOptions(ConfigParseOptions options) {
+        return new SimpleIncludeContext(parseable, options.setSyntax(null).setOriginDescription(null));
     }
 }

config/src/test/scala/com/typesafe/config/impl/ConcatenationTest.scala

@@ -235,7 +235,7 @@ class ConcatenationTest extends TestUtils {
         val e = intercept[ConfigException.Parse] {
             parseConfig("""{ { a : 1 } : "value" }""")
         }
-        assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("expecting a close") && e.getMessage.contains("'{'"))
+        assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("expecting a close parentheses") && e.getMessage.contains("'{'"))
     }
 
     @Test
@@ -243,7 +243,7 @@ class ConcatenationTest extends TestUtils {
         val e = intercept[ConfigException.Parse] {
             parseConfig("""{ [ "a" ] : "value" }""")
         }
-        assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("expecting a close") && e.getMessage.contains("'['"))
+        assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("expecting a close parentheses") && e.getMessage.contains("'['"))
     }
 
     @Test

config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala

@@ -5,10 +5,11 @@ package com.typesafe.config.impl
 
 import org.junit.Assert._
 import org.junit._
-import java.io.StringReader
+import java.io.{ File, StringReader }
+
 import com.typesafe.config._
+
 import scala.collection.JavaConverters._
-import java.io.File
 import java.net.URL
 import java.util.Properties
 
@@ -17,7 +18,7 @@ class ConfParserTest extends TestUtils {
     def parseWithoutResolving(s: String) = {
         val options = ConfigParseOptions.defaults().
             setOriginDescription("test conf string").
-            setSyntax(ConfigSyntax.CONF);
+            setSyntax(ConfigSyntax.CONF)
         Parseable.newString(s, options).parseValue().asInstanceOf[AbstractConfigValue]
     }
 
@@ -644,7 +645,7 @@ class ConfParserTest extends TestUtils {
         // properties-like syntax
         val conf8 = parseConfig("""
                 # ignored comment
-                
+
                 # x.y comment
                 x.y = 10
                 # x.z comment
@@ -709,29 +710,22 @@ class ConfParserTest extends TestUtils {
 
     @Test
     def includeFileNotQuoted() {
-        // this test cannot work on Windows
         val f = resourceFile("test01")
-        if (isWindows) {
-            System.err.println("includeFileNotQuoted test skipped on Windows")
-        } else {
-            val e = intercept[ConfigException.Parse] {
-                ConfigFactory.parseString("include file(" + f + ")")
-            }
-            assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("expecting include parameter"))
+        val e = intercept[ConfigException.Parse] {
+            ConfigFactory.parseString("include file(" + f + ")")
         }
+        assertTrue("wrong exception: " + e.getMessage,
+            e.getMessage.contains("expecting include file() parameter to be a quoted string"))
     }
 
     @Test
     def includeFileNotQuotedAndSpecialChar() {
         val f = resourceFile("test01")
-        if (isWindows) {
-            System.err.println("includeFileNotQuoted test skipped on Windows")
-        } else {
-            val e = intercept[ConfigException.Parse] {
-                ConfigFactory.parseString("include file(:" + f + ")")
-            }
-            assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("expecting a quoted string"))
+        val e = intercept[ConfigException.Parse] {
+            ConfigFactory.parseString("include file(:" + f + ")")
         }
+        assertTrue("wrong exception: " + e.getMessage,
+            e.getMessage.contains("expecting include file() parameter to be a quoted string, rather than: ':'"))
     }
 
     @Test
@@ -739,7 +733,7 @@ class ConfParserTest extends TestUtils {
         val e = intercept[ConfigException.Parse] {
             ConfigFactory.parseString("include file(" + jsonQuotedResourceFile("test01") + " something")
         }
-        assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("expecting a close paren"))
+        assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("expecting a close parentheses"))
     }
 
     @Test
@@ -779,6 +773,65 @@ class ConfParserTest extends TestUtils {
         assertEquals("abc", conf.getString("fromProps.abc"))
     }
 
+    @Test
+    def includeRequiredMissing() {
+        // set this to allowMissing=true to demonstrate that the missing inclusion causes failure despite this setting
+        val missing = ConfigParseOptions.defaults().setAllowMissing(true)
+
+        val ex = intercept[Exception] {
+            ConfigFactory.parseString("include required(classpath( \"nonexistant\") )", missing)
+        }
+
+        val actual = ex.getMessage
+        val expected = ".*resource not found on classpath.*"
+        assertTrue(s"expected match for <$expected> but got <$actual>", actual.matches(expected))
+    }
+
+    @Test
+    def includeRequiredFoundButNestedIncludeMissing() {
+        // set this to allowMissing=true to demonstrate that the missing nested inclusion is permitted despite this setting
+        val missing = ConfigParseOptions.defaults().setAllowMissing(false)
+
+        // test03 has a missing include
+        val conf = ConfigFactory.parseString("include required(classpath( \"test03\") )", missing)
+
+        val expected = "This is in the included file"
+        val actual = conf.getString("foo")
+        assertTrue(s"expected match for <$expected> but got <$actual>", actual.matches(expected))
+    }
+
+    @Test
+    def includeRequiredFound() {
+        val confs = Seq(
+            "include required(\"test01\")",
+            "include required( \"test01\" )",
+
+            "include required(classpath(\"test01\"))",
+            "include required( classpath(\"test01\"))",
+            "include required(classpath(\"test01\") )",
+            "include required( classpath(\"test01\") )",
+
+            "include required(classpath( \"test01\" ))",
+            "include required( classpath( \"test01\" ))",
+            "include required(classpath( \"test01\" ) )",
+            "include required( classpath( \"test01\" ) )"
+      )
+
+        // should have loaded conf, json, properties
+        confs.foreach { c =>
+            try {
+                val conf = ConfigFactory.parseString(c)
+                assertEquals(42, conf.getInt("ints.fortyTwo"))
+                assertEquals(1, conf.getInt("fromJson1"))
+                assertEquals("abc", conf.getString("fromProps.abc"))
+            } catch {
+                case ex: Exception =>
+                    System.err.println(s"failed parsing: $c")
+                    throw ex
+            }
+        }
+    }
+
     @Test
     def includeURLHeuristically() {
         val url = resourceFile("test01.conf").toURI().toURL().toExternalForm()

config/src/test/scala/com/typesafe/config/impl/TestUtils.scala

@@ -428,6 +428,7 @@ abstract trait TestUtils {
         "# bar\n", // just a comment with a newline
         "# foo\n//bar", // comment then another with no newline
         """{ "foo" = 42 }""", // equals rather than colon
+        """{ "foo" = (42) }""", // value with round braces
         """{ foo { "bar" : 42 } }""", // omit the colon for object value
         """{ foo baz { "bar" : 42 } }""", // omit the colon with unquoted key with spaces
         """ "foo" : 42 """, // omit braces on root object
@@ -721,8 +722,10 @@ abstract trait TestUtils {
 
     val resourceDir = {
         val f = new File("config/src/test/resources")
-        if (!f.exists())
-            throw new Exception("Tests must be run from the root project directory containing " + f.getPath())
+        if (!f.exists()) {
+            val here = new File(".").getAbsolutePath
+            throw new Exception(s"Tests must be run from the root project directory containing ${f.getPath()}, however the current directory is $here")
+        }
         f
     }
 

config/src/test/scala/com/typesafe/config/impl/TokenizerTest.scala

@@ -97,6 +97,13 @@ class TokenizerTest extends TestUtils {
         tokenizerTest(expected, source)
     }
 
+    @Test
+    def tokenizeUnquotedTextContainingRoundBrace() {
+        val source = """(footrue)"""
+        val expected = List(tokenUnquoted("(footrue)"))
+        tokenizerTest(expected, source)
+    }
+
     @Test
     def tokenizeUnquotedTextContainingTrue() {
         val source = """footrue"""
@@ -196,7 +203,7 @@ class TokenizerTest extends TestUtils {
         for (t <- invalidTests) {
             val tokenized = tokenizeAsList(t)
             val maybeProblem = tokenized.find(Tokens.isProblem(_))
-            assertTrue(maybeProblem.isDefined)
+            assertTrue(s"expected failure for <$t> but got ${t}", maybeProblem.isDefined)
         }
     }
 

config/src/test/scala/com/typesafe/config/impl/UnitParserTest.scala

@@ -31,13 +31,13 @@ class UnitParserTest extends TestUtils {
         val e = intercept[ConfigException.BadValue] {
             SimpleConfig.parseDuration("100 dollars", fakeOrigin(), "test")
         }
-        assertTrue(e.getMessage().contains("time unit"))
+        assertTrue(e.getMessage.contains("time unit"))
 
         // bad number
         val e2 = intercept[ConfigException.BadValue] {
             SimpleConfig.parseDuration("1 00 seconds", fakeOrigin(), "test")
         }
-        assertTrue(e2.getMessage().contains("duration number"))
+        assertTrue(e2.getMessage.contains("duration number"))
     }
 
     // https://github.com/typesafehub/config/issues/117
@@ -93,7 +93,7 @@ class UnitParserTest extends TestUtils {
         var result = 1024L * 1024 * 1024
         for (unit <- Seq("tebi", "pebi", "exbi")) {
             val first = unit.substring(0, 1).toUpperCase()
-            result = result * 1024;
+            result = result * 1024
             assertEquals(result, parseMem("1" + first))
             assertEquals(result, parseMem("1" + first + "i"))
             assertEquals(result, parseMem("1" + first + "iB"))
@@ -104,7 +104,7 @@ class UnitParserTest extends TestUtils {
         result = 1000L * 1000 * 1000
         for (unit <- Seq("tera", "peta", "exa")) {
             val first = unit.substring(0, 1).toUpperCase()
-            result = result * 1000;
+            result = result * 1000
             assertEquals(result, parseMem("1" + first + "B"))
             assertEquals(result, parseMem("1" + unit + "byte"))
             assertEquals(result, parseMem("1" + unit + "bytes"))
@@ -114,13 +114,13 @@ class UnitParserTest extends TestUtils {
         val e = intercept[ConfigException.BadValue] {
             SimpleConfig.parseBytes("100 dollars", fakeOrigin(), "test")
         }
-        assertTrue(e.getMessage().contains("size-in-bytes unit"))
+        assertTrue(e.getMessage.contains("size-in-bytes unit"))
 
         // bad number
         val e2 = intercept[ConfigException.BadValue] {
             SimpleConfig.parseBytes("1 00 bytes", fakeOrigin(), "test")
         }
-        assertTrue(e2.getMessage().contains("size-in-bytes number"))
+        assertTrue(e2.getMessage.contains("size-in-bytes number"))
     }
 
     // later on we'll want to check this with BigInteger version of getBytes