From b54deccfaebbd8449643da7518e2eb39d4fa1ee4 Mon Sep 17 00:00:00 2001 From: Ben Ogle Date: Mon, 29 Sep 2014 15:50:54 -0700 Subject: [PATCH] String type must be strict. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It makes sense to coerce from more general -> more specific data types. eg. string -> int, etc. But coercing the other way is problematic in the case of chaining because the more general type will swallow the specific type. eg. Setting `false` on type: [‘string’, ‘boolean’] will coerce the boolean to a string, and will never allow the value to be a boolean. --- spec/config-spec.coffee | 14 +++++++------- src/config.coffee | 10 ++++------ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/spec/config-spec.coffee b/spec/config-spec.coffee index 80d613b7e..450564ea2 100644 --- a/spec/config-spec.coffee +++ b/spec/config-spec.coffee @@ -701,21 +701,21 @@ describe "Config", -> atom.config.set('foo.bar.aString', 'yep') expect(atom.config.get('foo.bar.aString')).toBe 'yep' - it 'will only set strings, numbers and booleans', -> - expect(atom.config.set('foo.bar.aString', 123)).toBe true - expect(atom.config.get('foo.bar.aString')).toBe '123' + it 'will only set strings', -> + expect(atom.config.set('foo.bar.aString', 123)).toBe false + expect(atom.config.get('foo.bar.aString')).toBe 'ok' expect(atom.config.set('foo.bar.aString', true)).toBe false - expect(atom.config.get('foo.bar.aString')).toBe '123' + expect(atom.config.get('foo.bar.aString')).toBe 'ok' expect(atom.config.set('foo.bar.aString', null)).toBe false - expect(atom.config.get('foo.bar.aString')).toBe '123' + expect(atom.config.get('foo.bar.aString')).toBe 'ok' expect(atom.config.set('foo.bar.aString', [])).toBe false - expect(atom.config.get('foo.bar.aString')).toBe '123' + expect(atom.config.get('foo.bar.aString')).toBe 'ok' expect(atom.config.set('foo.bar.aString', nope: 'nope')).toBe false - expect(atom.config.get('foo.bar.aString')).toBe '123' + expect(atom.config.get('foo.bar.aString')).toBe 'ok' describe 'when the value has an "object" type', -> beforeEach -> diff --git a/src/config.coffee b/src/config.coffee index 9b4ee7c4b..b2eb74f1e 100644 --- a/src/config.coffee +++ b/src/config.coffee @@ -667,12 +667,10 @@ Config.addSchemaEnforcers throw new Error("Validation failed at #{keyPath}, #{JSON.stringify(value)} must be a boolean or the string 'true' or 'false'") 'string': - coerce: (keyPath, value, schema) -> - switch typeof value - when 'number', 'string' - value.toString() - else - throw new Error("Validation failed at #{keyPath}, #{JSON.stringify(value)} must be a string or number") + validate: (keyPath, value, schema) -> + unless typeof value is 'string' + throw new Error("Validation failed at #{keyPath}, #{JSON.stringify(value)} must be a string") + value 'null': # null sort of isnt supported. It will just unset in this case