implemented "required include" along with code review comments from p…

…revious attempt at pull request

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
@@ -1494,7 +1507,9 @@ environment variables generally are capitalized. This avoids
 naming collisions between environment variables and configuration
 properties. (While on Windows getenv() is generally not
 case-sensitive, the lookup will be case sensitive all the way
-until the env variable fallback lookup is reached.)
+until the env variable fallback lookup is reached).
+
+See also the notes below on Windows and case sensitivity.
 
 An application can explicitly block looking up a substitution in
 the environment by setting a value in the configuration, with the
@@ -1543,3 +1558,47 @@ Differences include but are probably not limited to:
    properties files only recognize comment characters if they
    occur as the first character on the line
  - HOCON interprets `${}` as a substitution
+
+## Note on Windows and case sensitivity of environment variables
+
+HOCON's lookup of environment variable values is always case sensitive, but
+Linux and Windows differ in their handling of case.
+
+Linux allows one to define multiple environment variables with the same
+name but with different case; so both "PATH" and "Path" may be defined
+simultaneously. HOCON's access to these environment variables on Linux
+is straightforward; ie just make sure you define all your vars with the required case.
+
+Windows is more confusing. Windows environment variables names may contain a
+mix of upper and lowercase characters, eg "Path", however Windows does not
+allow one to define multiple instances of the same name but differing in case.
+Whilst accessing env vars in Windows is case insensitive, accessing env vars in
+HOCON is case sensitive.
+So if you know that you HOCON needs "PATH" then you must ensure that
+the variable is defined as "PATH" rather than some other name such as
+"Path" or "path".
+However, Windows does not allow us to change the case of an existing env var; we can't
+simply redefine the var with an upper case name.
+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 definition ...
+
+```
+set Path=A;B;C
+```
+.. we just don't know. But if the HOCON needs "PATH", then the start script must
+take a precautionary approach and enforce the necessary case as follows ...
+
+```
+set OLDPATH=%PATH%
+set PATH=
+set PATH=%OLDPATH%
+
+%JAVA_HOME%/bin/java ....
+```
+
+You cannot know what ambient environment variables might exist in the ambient environment
+when your program is invoked, nor what case those definitions might have.
+Therefore the only safe thing to do is redefine all the vars you rely on as shown above.

README.md

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

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

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

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

@@ -111,7 +111,8 @@ ConfigParseOptions withFallbackOriginDescription(String originDescription) {
     /**
      * 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 @@ private ConfigNodePath parseKey(Token token) {
                 }
 
                 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,25 +310,74 @@ private boolean isKeyValueSeparatorToken(Token t) {
             }
         }
 
+        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
             if (Tokens.isUnquotedText(t)) {
                 // 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 @@ private ConfigNodeInclude parseInclude(ArrayList<AbstractConfigNode> children) {
 
                 // 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 @@
 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 @@ protected ConfigIncludeKind kind() {
         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 @@ Collections.<String, AbstractConfigValue> singletonMap(
         }
 
         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 @@ private void parseInclude(Map<String, AbstractConfigValue> values, ConfigNodeInc
                     } 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 @@
 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 @@ public ConfigParseable relativeTo(String filename) {
 
     @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,15 +235,15 @@ 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
     def arraysAreNotKeys() {
         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