remove special-case fallback to system props in substitutions.

The earlier change to make ${user.home} in a file included underneath
"foo" search both ${foo.user.home} and then ${user.home}, means
that even in included files system props are picked up just fine
as long as they were merged into the root config. So there is no
longer any need to have a special-case fallback to system properties.

This leaves ConfigResolveOptions as a really complex way to pass
in a single bool, but of course the point is to allow for future
extension.

HOCON.md

@@ -392,11 +392,6 @@ path expression in a key, because it has a special interpretation
 Substitutions are a way of referring to other parts of the
 configuration tree.
 
-For substitutions which are not found in the configuration tree,
-implementations may try to resolve them by looking at system
-environment variables, Java system properties, or other external
-sources of configuration.
-
 The syntax is `${pathexpression}` or `${?pathexpression}` where
 the `pathexpression` is a path expression as described above. This
 path expression has the same syntax that you could use for an
@@ -406,6 +401,11 @@ The `?` in `${?pathexpression}` must not have whitespace before
 it; the three characters `${?` must be exactly like that, grouped
 together.
 
+For substitutions which are not found in the configuration tree,
+implementations may try to resolve them by looking at system
+environment variables or other external sources of configuration.
+(More detail on environment variables in a later section.)
+
 Substitutions are not parsed inside quoted strings. To get a
 string containing a substitution, you must use value concatenation
 with the substitution in the unquoted portion:
@@ -428,11 +428,6 @@ more than once, the substitution will always evaluate to its
 latest-assigned value (the merged object or the last non-object
 value that was set).
 
-If a substitutions does not match any value present in the
-configuration, implementations may look up that substitution in
-one or more external sources, such as a Java system property or an
-environment variable. (More detail on this in a later section.)
-
 If a configuration sets a value to `null` then it should not be
 looked up in the external source. Unfortunately there is no way to
 "undo" this in a later configuration file; if you have `{ "HOME" :
@@ -962,31 +957,12 @@ For an application's config, Java system properties _override_
 settings found in the configuration file. This supports specifying
 config options on the command line.
 
-### Substitution fallback to system properties
+### Substitution fallback to environment variables
 
 Recall that if a substitution is not present (not even set to
 `null`) within a configuration tree, implementations may search
-for it from external sources. One such source could be Java system
-properties.
-
-If you merge system properties in to an application's
-configuration as overrides, then falling back to them for
-substitutions isn't important when parsing a toplevel config file
-- they would already be found in the configuration.
-
-However, there are some cases when system properties fallbacks are
-used. For example, when a config file is parsed as an include, its
-substitutions are relative to the key the file is included
-underneath. In that case, `${user.home}` might become
-`${something.user.home}` and would not match a system property
-override applied to the root of the config tree. However,
-`${user.home}` always falls back to the system property.
-
-### Substitution fallback to environment variables
-
-Substitutions not found in the configuration may also fall back to
-environment variables. In Java, fallback should be to system
-properties first and environment variables second.
+for it from external sources. One such source could be environment
+variables.
 
 It's recommended that HOCON keys always use lowercase, because
 environment variables generally are capitalized. This avoids

README.md

@@ -22,7 +22,7 @@ Configuration library for JVM languages.
     - substitutions (`"foo" : ${bar}`, `"foo" : Hello ${who}`)
     - properties-like notation (`a.b=c`)
     - less noisy, more lenient syntax
-    - substitute environment variables and system properties
+    - substitute environment variables
 
 This library limits itself to config files. If you want to load
 config from a database or something, you would need to build a
@@ -116,10 +116,10 @@ detail.
     as the `b` field in the `a` object
   - substitutions concatenate into unquoted strings, `foo : the
     quick ${colors.fox} jumped`
-  - substitutions fall back to system properties and then
-    environment variables if they don't resolve in the
-    config itself, so `${HOME}` or `${user.home}` would
-    work as you expect.
+  - substitutions fall back to environment variables if they don't
+    resolve in the config itself, so `${HOME}` would work as you
+    expect. Also, most configs have system properties merged in so
+    you could use `${user.home}`.
   - substitutions normally cause an error if unresolved, but
     there is a syntax `${?a.b}` to permit them to be missing.
 
@@ -257,10 +257,10 @@ Here are some features that might be nice to add.
    object merge you have to first set the object to a non-object
    such as null, then set a new object.)
  - "delete": allow deleting a field, which is slightly different
-   from setting it to null (deletion allows fallback to values
-   in system properties and the environment, for example).
-   This could be done using the same syntax as `include`,
-   potentially. It is not a backward-compatible change though.
+   from setting it to null (deletion allows fallback to values in
+   the environment, for example).  This could be done using the
+   same syntax as `include`, potentially. It is not a
+   backward-compatible change though.
  - substitutions with fallbacks; this could be something like
    `${foo.bar,baz,null}` where it would look up `foo.bar`, then
    `baz`, then finally fall back to null. One question is whether

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

@@ -57,17 +57,6 @@ public final class ConfigFactory {
     /**
      * Like {@link #load(String)} but allows you to specify parse and resolve
      * options.
-     * <p>
-     * To be aware of: using
-     * {@link ConfigResolveOptions#setUseSystemProperties(boolean)
-     * setUseSystemProperties(false)} with this method has little effect,
-     * because the system properties are merged into the config as overrides
-     * anyway. <code>setUseSystemProperties</code> affects whether to fall back
-     * to system properties when they are not found in the config, but with
-     * <code>load()</code>, they will be in the config. There is one situation
-     * where <code>setUseSystemProperties(false)</code> comes into play, which
-     * is that files included into another file may find the system property
-     * fallbacks but not the overrides.
      * @param resourceBasename
      *            the classpath resource name with optional extension
      * @param parseOptions
@@ -100,17 +89,6 @@ public final class ConfigFactory {
     /**
      * Like {@link #load(Config)} but allows you to specify
      * {@link ConfigResolveOptions}.
-     * <p>
-     * To be aware of: using
-     * {@link ConfigResolveOptions#setUseSystemProperties(boolean)
-     * setUseSystemProperties(false)} with this method has little effect,
-     * because the system properties are merged into the config as overrides
-     * anyway. <code>setUseSystemProperties</code> affects whether to fall back
-     * to system properties when they are not found in the config, but with
-     * <code>load()</code>, they will be in the config. There is one situation
-     * where <code>setUseSystemProperties(false)</code> comes into play, which
-     * is that files included into another file may find the system property
-     * fallbacks but not the overrides.
      * 
      * @param config
      *            the application's portion of the configuration

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

@@ -7,31 +7,24 @@ package com.typesafe.config;
  * A set of options related to resolving substitutions. Substitutions use the
  * <code>${foo.bar}</code> syntax and are documented in the <a
  * href="https://github.com/havocp/config/blob/master/HOCON.md">HOCON</a> spec.
- *
  * <p>
  * This object is immutable, so the "setters" return a new object.
- *
  * <p>
  * Here is an example of creating a custom {@code ConfigResolveOptions}:
- *
  * <pre>
  *     ConfigResolveOptions options = ConfigResolveOptions.defaults()
- *         .setUseSystemProperties(false)
  *         .setUseSystemEnvironment(false)
  * </pre>
- *
  * <p>
  * In addition to {@link ConfigResolveOptions#defaults}, there's a prebuilt
  * {@link ConfigResolveOptions#noSystem} which avoids looking at any system
- * properties or environment variables.
+ * environment variables or other external system information. (Right now,
+ * environment variables are the only example.)
  */
 public final class ConfigResolveOptions {
-    private final boolean useSystemProperties;
     private final boolean useSystemEnvironment;
 
-    private ConfigResolveOptions(boolean useSystemProperties,
-            boolean useSystemEnvironment) {
-        this.useSystemProperties = useSystemProperties;
+    private ConfigResolveOptions(boolean useSystemEnvironment) {
         this.useSystemEnvironment = useSystemEnvironment;
     }
 
@@ -41,31 +34,17 @@ public final class ConfigResolveOptions {
      * @return the default resolve options
      */
     public static ConfigResolveOptions defaults() {
-        return new ConfigResolveOptions(true, true);
+        return new ConfigResolveOptions(true);
     }
 
     /**
      * Returns resolve options that disable any reference to "system" data
-     * (system properties or environment variables).
+     * (currently, this means environment variables).
      *
-     * @return the resolve options with system properties and env variables
-     *         disabled
+     * @return the resolve options with env variables disabled
      */
     public static ConfigResolveOptions noSystem() {
-        return defaults().setUseSystemEnvironment(false).setUseSystemProperties(false);
-    }
-
-    /**
-     * Returns options with use of Java system properties set to the given
-     * value.
-     *
-     * @param value
-     *            true to resolve substitutions falling back to Java system
-     *            properties.
-     * @return options with requested setting for use of system properties
-     */
-    public ConfigResolveOptions setUseSystemProperties(boolean value) {
-        return new ConfigResolveOptions(value, useSystemEnvironment);
+        return defaults().setUseSystemEnvironment(false);
     }
 
     /**
@@ -76,18 +55,9 @@ public final class ConfigResolveOptions {
      *            variables.
      * @return options with requested setting for use of environment variables
      */
+    @SuppressWarnings("static-method")
     public ConfigResolveOptions setUseSystemEnvironment(boolean value) {
-        return new ConfigResolveOptions(useSystemProperties, value);
-    }
-
-    /**
-     * Returns whether the options enable use of system properties. This method
-     * is mostly used by the config lib internally, not by applications.
-     *
-     * @return true if system properties should be used
-     */
-    public boolean getUseSystemProperties() {
-        return useSystemProperties;
+        return new ConfigResolveOptions(value);
     }
 
     /**

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

@@ -146,11 +146,6 @@ final class ConfigSubstitution extends AbstractConfigValue implements
                 result = findInObject(resolver.root(), resolver, unprefixed, depth, options);
             }
 
-            if (result == null && options.getUseSystemProperties()) {
-                result = findInObject(ConfigImpl.systemPropertiesAsConfigObject(), null,
-                        unprefixed, depth, options);
-            }
-
             if (result == null && options.getUseSystemEnvironment()) {
                 result = findInObject(ConfigImpl.envVariablesAsConfigObject(), null, unprefixed,
                         depth, options);

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

@@ -322,13 +322,16 @@ class ConfigSubstitutionTest extends TestUtils {
 """)
     }
 
+    // this is a weird test, it used to test fallback to system props which made more sense.
+    // Now it just tests that if you override with system props, you can use system props
+    // in substitutions.
     @Test
-    def fallbackToSystemProps() {
+    def overrideWithSystemProps() {
         System.setProperty("configtest.a", "1234")
         System.setProperty("configtest.b", "5678")
         ConfigImpl.reloadSystemPropertiesConfig()
 
-        val resolved = resolve(substSystemPropsObject)
+        val resolved = resolve(ConfigFactory.systemProperties().withFallback(substSystemPropsObject).root.asInstanceOf[AbstractConfigObject])
 
         assertEquals("1234", resolved.getString("a"))
         assertEquals("5678", resolved.getString("b"))