Merge pull request #5280 from atom/bo-schema-on-load

Reset all user config values when the schema changes

spec/config-spec.coffee

@@ -699,6 +699,26 @@ describe "Config", ->
           expect(atom.config.get("foo.bar")).toBe 'baz'
           expect(atom.config.get("foo.bar", scope: ['.source.ruby'])).toBe 'more-specific'
 
+      describe "when the config file does not conform to the schema", ->
+        beforeEach ->
+          fs.writeFileSync atom.config.configFilePath, """
+            '*':
+              foo:
+                bar: 'omg'
+                int: 'baz'
+            '.source.ruby':
+              foo:
+                bar: 'scoped'
+                int: 'nope'
+          """
+
+        it "validates and does not load the incorrect values", ->
+          atom.config.loadUserConfig()
+          expect(atom.config.get("foo.int")).toBe 12
+          expect(atom.config.get("foo.bar")).toBe 'omg'
+          expect(atom.config.get("foo.int", scope: ['.source.ruby'])).toBe 12
+          expect(atom.config.get("foo.bar", scope: ['.source.ruby'])).toBe 'scoped'
+
       describe "when the config file contains valid cson", ->
         beforeEach ->
           fs.writeFileSync(atom.config.configFilePath, "foo: bar: 'baz'")
@@ -1106,6 +1126,62 @@ describe "Config", ->
         expect(atom.config.get('foo.bar.str', scope: ['.source.js'])).toBe 'omg'
         expect(atom.config.get('foo.bar.str', scope: ['.source.coffee'])).toBe 'ok'
 
+      describe 'when a schema is added after config values have been set', ->
+        schema = null
+        beforeEach ->
+          schema =
+            type: 'object'
+            properties:
+              int:
+                type: 'integer'
+                default: 2
+              str:
+                type: 'string'
+                default: 'def'
+
+        it "respects the new schema when values are set", ->
+          expect(atom.config.set('foo.bar.str', 'global')).toBe true
+          expect(atom.config.set('foo.bar.str', 'scoped', scopeSelector: '.source.js')).toBe true
+          expect(atom.config.get('foo.bar.str')).toBe 'global'
+          expect(atom.config.get('foo.bar.str', scope: ['.source.js'])).toBe 'scoped'
+
+          expect(atom.config.set('foo.bar.noschema', 'nsGlobal')).toBe true
+          expect(atom.config.set('foo.bar.noschema', 'nsScoped', scopeSelector: '.source.js')).toBe true
+          expect(atom.config.get('foo.bar.noschema')).toBe 'nsGlobal'
+          expect(atom.config.get('foo.bar.noschema', scope: ['.source.js'])).toBe 'nsScoped'
+
+          expect(atom.config.set('foo.bar.int', 'nope')).toBe true
+          expect(atom.config.set('foo.bar.int', 'notanint', scopeSelector: '.source.js')).toBe true
+          expect(atom.config.set('foo.bar.int', 23, scopeSelector: '.source.coffee')).toBe true
+          expect(atom.config.get('foo.bar.int')).toBe 'nope'
+          expect(atom.config.get('foo.bar.int', scope: ['.source.js'])).toBe 'notanint'
+          expect(atom.config.get('foo.bar.int', scope: ['.source.coffee'])).toBe 23
+
+          atom.config.setSchema('foo.bar', schema)
+
+          expect(atom.config.get('foo.bar.str')).toBe 'global'
+          expect(atom.config.get('foo.bar.str', scope: ['.source.js'])).toBe 'scoped'
+          expect(atom.config.get('foo.bar.noschema')).toBe 'nsGlobal'
+          expect(atom.config.get('foo.bar.noschema', scope: ['.source.js'])).toBe 'nsScoped'
+
+          expect(atom.config.get('foo.bar.int')).toBe 2
+          expect(atom.config.get('foo.bar.int', scope: ['.source.js'])).toBe 2
+          expect(atom.config.get('foo.bar.int', scope: ['.source.coffee'])).toBe 23
+
+        it "sets all values that adhere to the schema", ->
+          expect(atom.config.set('foo.bar.int', 10)).toBe true
+          expect(atom.config.set('foo.bar.int', 15, scopeSelector: '.source.js')).toBe true
+          expect(atom.config.set('foo.bar.int', 23, scopeSelector: '.source.coffee')).toBe true
+          expect(atom.config.get('foo.bar.int')).toBe 10
+          expect(atom.config.get('foo.bar.int', scope: ['.source.js'])).toBe 15
+          expect(atom.config.get('foo.bar.int', scope: ['.source.coffee'])).toBe 23
+
+          atom.config.setSchema('foo.bar', schema)
+
+          expect(atom.config.get('foo.bar.int')).toBe 10
+          expect(atom.config.get('foo.bar.int', scope: ['.source.js'])).toBe 15
+          expect(atom.config.get('foo.bar.int', scope: ['.source.coffee'])).toBe 23
+
       describe 'when the value has an "integer" type', ->
         beforeEach ->
           schema =

src/config.coffee

@@ -602,7 +602,7 @@ class Config
         return false
 
     if scopeSelector?
-      @setRawScopedValue(source, scopeSelector, keyPath, value)
+      @setRawScopedValue(keyPath, value, source, scopeSelector)
     else
       @setRawValue(keyPath, value)
 
@@ -797,6 +797,7 @@ class Config
     _.extend rootSchema, schema
     @setDefaults(keyPath, @extractDefaultsFromSchema(schema))
     @setScopedDefaultsFromSchema(keyPath, schema)
+    @resetSettingsForSchemaChange()
 
   load: ->
     @initializeConfigDirectory()
@@ -998,9 +999,28 @@ class Config
       defaults[key] = @extractDefaultsFromSchema(value) for key, value of properties
       defaults
 
-  makeValueConformToSchema: (keyPath, value) ->
-    value = @constructor.executeSchemaEnforcers(keyPath, value, schema) if schema = @getSchema(keyPath)
-    value
+  makeValueConformToSchema: (keyPath, value, options) ->
+    if options?.suppressException
+      try
+        @makeValueConformToSchema(keyPath, value)
+      catch e
+        undefined
+    else
+      value = @constructor.executeSchemaEnforcers(keyPath, value, schema) if schema = @getSchema(keyPath)
+      value
+
+  # When the schema is changed / added, there may be values set in the config
+  # that do not conform to the schema. This will reset make them conform.
+  resetSettingsForSchemaChange: (source=@getUserConfigPath()) ->
+    @transact =>
+      @settings = @makeValueConformToSchema(null, @settings, suppressException: true)
+      priority = @priorityForSource(source)
+      selectorsAndSettings = @scopedSettingsStore.propertiesForSource(source)
+      @scopedSettingsStore.removePropertiesForSource(source)
+      for scopeSelector, settings of selectorsAndSettings
+        settings = @makeValueConformToSchema(null, settings, suppressException: true)
+        @setRawScopedValue(null, settings, source, scopeSelector)
+      return
 
   ###
   Section: Private Scoped Settings
@@ -1017,8 +1037,15 @@ class Config
 
   resetUserScopedSettings: (newScopedSettings) ->
     source = @getUserConfigPath()
+    priority = @priorityForSource(source)
     @scopedSettingsStore.removePropertiesForSource(source)
-    @scopedSettingsStore.addProperties(source, newScopedSettings, priority: @priorityForSource(source))
+
+    for scopeSelector, settings of newScopedSettings
+      settings = @makeValueConformToSchema(null, settings, suppressException: true)
+      validatedSettings = {}
+      validatedSettings[scopeSelector] = withoutEmptyObjects(settings)
+      @scopedSettingsStore.addProperties(source, validatedSettings, {priority}) if validatedSettings[scopeSelector]?
+
     @emitChangeEvent()
 
   addScopedSettings: (source, selector, value, options) ->
@@ -1031,7 +1058,7 @@ class Config
       disposable.dispose()
       @emitChangeEvent()
 
-  setRawScopedValue: (source, selector, keyPath, value) ->
+  setRawScopedValue: (keyPath, value, source, selector, options) ->
     if keyPath?
       newValue = {}
       _.setValueForKeyPath(newValue, keyPath, value)
@@ -1118,12 +1145,17 @@ Config.addSchemaEnforcers
       return value unless schema.properties?
 
       newValue = {}
-      for prop, childSchema of schema.properties
-        continue unless value.hasOwnProperty(prop)
-        try
-          newValue[prop] = @executeSchemaEnforcers("#{keyPath}.#{prop}", value[prop], childSchema)
-        catch error
-          console.warn "Error setting item in object: #{error.message}"
+      for prop, propValue of value
+        childSchema = schema.properties[prop]
+        if childSchema?
+          try
+            newValue[prop] = @executeSchemaEnforcers("#{keyPath}.#{prop}", propValue, childSchema)
+          catch error
+            console.warn "Error setting item in object: #{error.message}"
+        else
+          # Just pass through un-schema'd values
+          newValue[prop] = propValue
+
       newValue
 
   'array':