From 5e2ef371e943d7cfca7b341b687119a1a7aea6b4 Mon Sep 17 00:00:00 2001 From: Pierre-Antoine Rousseau Date: Sun, 6 Dec 2020 15:07:57 -0500 Subject: [PATCH 1/4] feat(options-v2): Improve options validation for svelte v2 parser (#29) --- lib/parser.js | 174 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 122 insertions(+), 52 deletions(-) diff --git a/lib/parser.js b/lib/parser.js index 6137d8d..21a6439 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -8,27 +8,40 @@ const jsdoc = require('./jsdoc'); const hasOwnProperty = utils.hasOwnProperty; -const DEFAULT_OPTIONS = { - /** - * Flag, indicating that source locations should be extracted from source. - */ - includeSourceLocations: false, - range: false, - // comment: true, - attachComment: true, - - // create a top-level tokens array containing all tokens - tokens: true, - - // The version of ECMAScript syntax to use - ecmaVersion: 9, - - // Type of script to parse - sourceType: 'module', - - ecmaFeatures: { - } -}; +/** + * @link https://github.com/eslint/espree#options + */ +function getAstDefaultOptions() { + return { + /** attach range information to each node */ + range: true, + + /** attach line/column location information to each node */ + loc: true, + + /** create a top-level comments array containing all comments */ + comment: true, + + /** create a top-level tokens array containing all tokens */ + tokens: true, + + /** + * Set to 3, 5 (default), 6, 7, 8, 9, 10, 11, or 12 to specify + * the version of ECMAScript syntax you want to use. + * + * You can also set to 2015 (same as 6), 2016 (same as 7), + * 2017 (same as 8), 2018 (same as 9), 2019 (same as 10), + * 2020 (same as 11), or 2021 (same as 12) to use the year-based naming. + */ + ecmaVersion: 9, + + /** specify which type of script you're parsing ("script" or "module") */ + sourceType: 'module', + + /** specify additional language features */ + ecmaFeatures: {} + }; +} const SUPPORTED_FEATURES = [ 'name', @@ -46,25 +59,32 @@ const SUPPORTED_FEATURES = [ 'refs', 'store' ]; +const isFeatureSupported = (v) => SUPPORTED_FEATURES.includes(v); + +const INFO_FEATURES_SUPPORTED = `Supported features: ${JSON.stringify(SUPPORTED_FEATURES)}`; + +function getUnsupporteFeaturesString(arr) { + return `Features [${utils.printArray(arr)}] in ` + + 'options.features are not supported by the SvelteV2 Parser. ' + + INFO_FEATURES_SUPPORTED; +} const EVENT_EMIT_RE = /\bfire\s*\(\s*((?:'[^']*')|(?:"[^"]*")|(?:`[^`]*`))/; class Parser extends EventEmitter { constructor(options) { - options = Object.assign({}, DEFAULT_OPTIONS, options); + super(); Parser.validateOptions(options); - super(); - this.source = options.source; - this.features = options.features || SUPPORTED_FEATURES; + this.features = options.features; if (hasOwnProperty(options.source, 'script') && options.source.script) { this.scriptOffset = options.source.scriptOffset || 0; try { - this.ast = espree.parse(options.source.script, options); + this.ast = espree.parse(options.source.script, options.ast); this.sourceCode = new eslint.SourceCode({ text: options.source.script, @@ -73,11 +93,14 @@ class Parser extends EventEmitter { } catch (e) { const script = utils.escapeImportKeyword(options.source.script); - this.ast = espree.parse(script, Object.assign({}, options, { + const newAstOptions = { + ...options.ast, loc: true, range: true, comment: true - })); + }; + + this.ast = espree.parse(script, newAstOptions); this.sourceCode = new eslint.SourceCode({ text: script, @@ -91,31 +114,86 @@ class Parser extends EventEmitter { } this.includeSourceLocations = options.includeSourceLocations; - this.componentName = null; + this.componentName = options.componentName; this.template = options.source.template; this.filename = options.filename; - this.eventsEmmited = {}; + this.eventsEmmited = options.eventsEmmited; this.defaultMethodVisibility = options.defaultMethodVisibility; this.defaultActionVisibility = options.defaultActionVisibility; - this.identifiers = {}; - this.imports = {}; + this.identifiers = options.identifiers; + this.imports = options.imports; + } + + static getDefaultOptions() { + return { + includeSourceLocations: true, + defaultMethodVisibility: utils.DEFAULT_VISIBILITY, + defaultActionVisibility: utils.DEFAULT_VISIBILITY, + features: [...SUPPORTED_FEATURES], + componentName: null, + eventsEmmited: {}, + identifiers: {}, + imports: {}, + ast: getAstDefaultOptions(), + }; + } + + static normalizeOptions(options) { + const defaults = Parser.getDefaultOptions(); + + Object.keys(defaults).forEach((optionKey) => { + // If the key was not set by the user, apply default value. + if (!(optionKey in options)) { + options[optionKey] = defaults[optionKey]; + } + }); } static validateOptions(options) { - if (!options.source) { - throw new Error('options.source is required'); + Parser.normalizeOptions(options); + + // Check the presence and basic format of multiple options + for (const key of ['eventsEmmited', 'identifiers', 'imports']) { + const hasKey = (key in options); + + if (!hasKey) { + throw new Error(`options.${key} is required`); + } + + const hasCorrectType = typeof options[key] === 'object'; + + if (!hasCorrectType) { + throw new TypeError(`options.${key} must be of type 'object'`); + } + } + + if ('source' in options) { + ['script', 'scriptOffset'].forEach(key => { + if (!(key in options.source)) { + throw new TypeError('options.source must have keys \'script\' and \'scriptOffset\''); + } + }); } - if (options.features) { + if ('features' in options) { if (!Array.isArray(options.features)) { throw new TypeError('options.features must be an array'); } - options.features.forEach((feature) => { - if (!SUPPORTED_FEATURES.includes(feature)) { - throw new Error(`Unknow '${feature}' feature. Supported features: ` + JSON.stringify(SUPPORTED_FEATURES)); - } - }); + if (options.features.length === 0) { + throw new Error( + 'options.features must contain at least one feature. ' + + INFO_FEATURES_SUPPORTED + ); + } + + if (!options.features.every(isFeatureSupported)) { + const notSupported = options.features.filter( + (iv) => !isFeatureSupported(iv) + ); + + throw new Error(getUnsupporteFeaturesString(notSupported)); + } } } @@ -379,19 +457,15 @@ class Parser extends EventEmitter { } internalWalk() { - if (this.features.length === 0) { - return this.emit('end'); - } - if (this.template) { this.parseTemplate(); } - if (this.ast === null) { - if (this.features.includes('name')) { - this.parseComponentName(); - } + if (this.features.includes('name')) { + this.parseComponentName(); + } + if (this.ast === null) { return this.emit('end'); } @@ -450,10 +524,6 @@ class Parser extends EventEmitter { } else if (body.expression !== null && body.expression.right && body.expression.right.properties) { body.expression.right.properties.forEach((property) => this.extractProperties(property)); } - - if (this.features.includes('name')) { - this.parseComponentName(); - } }); this.emit('end'); From 52b6371c51664a54cdead27764201254a56de54e Mon Sep 17 00:00:00 2001 From: Pierre-Antoine Rousseau Date: Mon, 7 Dec 2020 17:14:27 -0500 Subject: [PATCH 2/4] feat(options-v3): Improve options validation for svelte v3 parser (#29) - refactor: move features validation to lib/options - refactor: move ast options to lib/options - test: Add tests for options.validateFeatures - test: Add integration test for features validation --- lib/options.js | 112 ++++++++++++++++++++++++--- lib/parser.js | 90 ++++----------------- lib/v3/parser.js | 86 +++++++++++++------- test/integration/parse/parse.spec.js | 24 +++++- test/unit/options/options.spec.js | 90 ++++++++++++++++++--- 5 files changed, 276 insertions(+), 126 deletions(-) diff --git a/lib/options.js b/lib/options.js index dbbe408..0122f0b 100644 --- a/lib/options.js +++ b/lib/options.js @@ -34,7 +34,7 @@ function getUnsupportedVisibilitiesString(arr) { INFO_VISIBILITIES_SUPPORTED; } -const ErrorMessage = Object.freeze({ +const OptionsError = Object.freeze({ OptionsRequired: 'An options object is required.', InputRequired: 'One of options.filename or options.fileContent is required.', EncodingMissing: 'Internal Error: options.encoding is not set.', @@ -94,7 +94,7 @@ function normalize(options) { */ function validate(options) { if (!options) { - throw new Error(ErrorMessage.OptionsRequired); + throw new Error(OptionsError.OptionsRequired); } normalize(options); @@ -110,25 +110,25 @@ function validate(options) { isString(options.fileContent); if (!hasFilename && !hasFileContent) { - throw new Error(ErrorMessage.InputRequired); + throw new Error(OptionsError.InputRequired); } if ('encoding' in options) { if (!isString(options.encoding)) { - throw new Error(ErrorMessage.EncodingFormat); + throw new Error(OptionsError.EncodingFormat); } if (!ENCODINGS.includes(options.encoding)) { - throw new Error(ErrorMessage.EncodingNotSupported(options.encoding)); + throw new Error(OptionsError.EncodingNotSupported(options.encoding)); } } else { // Sanity check. At this point, 'encoding' should be set. - throw new Error(ErrorMessage.EncodingMissing); + throw new Error(OptionsError.EncodingMissing); } if ('ignoredVisibilities' in options) { if (!Array.isArray(options.ignoredVisibilities)) { - throw new Error(ErrorMessage.IgnoredVisibilitiesFormat); + throw new Error(OptionsError.IgnoredVisibilitiesFormat); } if (!options.ignoredVisibilities.every(isVisibilitySupported)) { @@ -136,16 +136,104 @@ function validate(options) { (iv) => !isVisibilitySupported(iv) ); - throw new Error(ErrorMessage.IgnoredVisibilitiesNotSupported(notSupported)); + throw new Error(OptionsError.IgnoredVisibilitiesNotSupported(notSupported)); } } else { // Sanity check. At this point, 'ignoredVisibilities' should be set. - throw new Error(ErrorMessage.IgnoredVisibilitiesMissing); + throw new Error(OptionsError.IgnoredVisibilitiesMissing); } } +const getSupportedFeaturesString = (supported) => `Supported features: ${printArray(supported)}`; + +const getFeaturesEmptyString = (supported) => { + return 'options.features must contain at least one feature. ' + + getSupportedFeaturesString(supported); +}; + +/** + * @param {string[]} notSupported + * @param {string[]} supported + */ +const getFeaturesNotSupportedString = (notSupported, supported) => { + return `Features [${printArray(notSupported)}] in ` + + 'options.features are not supported by this Parser. ' + + getSupportedFeaturesString(supported); +}; + +const ParserError = { + FeaturesMissing: 'Internal Error: options.features is not set.', + FeaturesFormat: 'options.features must be an array', + FeaturesEmpty: getFeaturesEmptyString, + FeaturesNotSupported: getFeaturesNotSupportedString, +}; + +/** + * + * @param {*} options + * @param {string[]} supported + * @throws if any validation fails for options.features + */ +function validateFeatures(options, supported) { + if ('features' in options) { + if (!Array.isArray(options.features)) { + throw new TypeError(ParserError.FeaturesFormat); + } + + if (options.features.length === 0) { + throw new Error(ParserError.FeaturesEmpty(supported)); + } + + const notSupported = options.features.filter((iv) => !supported.includes(iv)); + + if (notSupported.length > 0) { + throw new Error(ParserError.FeaturesNotSupported(notSupported, supported)); + } + } else { + throw new Error(ParserError.FeaturesMissing); + } +} + +/** + * @link https://github.com/eslint/espree#options + */ +function getAstDefaultOptions() { + return { + /** attach range information to each node */ + range: true, + + /** attach line/column location information to each node */ + loc: true, + + /** create a top-level comments array containing all comments */ + comment: true, + + /** create a top-level tokens array containing all tokens */ + tokens: true, + + /** + * Set to 3, 5 (default), 6, 7, 8, 9, 10, 11, or 12 to specify + * the version of ECMAScript syntax you want to use. + * + * You can also set to 2015 (same as 6), 2016 (same as 7), + * 2017 (same as 8), 2018 (same as 9), 2019 (same as 10), + * 2020 (same as 11), or 2021 (same as 12) to use the year-based naming. + */ + ecmaVersion: 9, + + /** specify which type of script you're parsing ("script" or "module") */ + sourceType: 'module', + + /** specify additional language features */ + ecmaFeatures: {} + }; +} + module.exports = { - ErrorMessage: ErrorMessage, - validate: validate, - retrieveFileOptions: retrieveFileOptions, + OptionsError, + ParserError, + validate, + validateFeatures, + retrieveFileOptions, + getAstDefaultOptions, }; diff --git a/lib/parser.js b/lib/parser.js index 21a6439..4473090 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -5,44 +5,10 @@ const HtmlParser = require('htmlparser2-svelte').Parser; const path = require('path'); const utils = require('./utils'); const jsdoc = require('./jsdoc'); +const { validateFeatures, getAstDefaultOptions } = require('./options'); const hasOwnProperty = utils.hasOwnProperty; -/** - * @link https://github.com/eslint/espree#options - */ -function getAstDefaultOptions() { - return { - /** attach range information to each node */ - range: true, - - /** attach line/column location information to each node */ - loc: true, - - /** create a top-level comments array containing all comments */ - comment: true, - - /** create a top-level tokens array containing all tokens */ - tokens: true, - - /** - * Set to 3, 5 (default), 6, 7, 8, 9, 10, 11, or 12 to specify - * the version of ECMAScript syntax you want to use. - * - * You can also set to 2015 (same as 6), 2016 (same as 7), - * 2017 (same as 8), 2018 (same as 9), 2019 (same as 10), - * 2020 (same as 11), or 2021 (same as 12) to use the year-based naming. - */ - ecmaVersion: 9, - - /** specify which type of script you're parsing ("script" or "module") */ - sourceType: 'module', - - /** specify additional language features */ - ecmaFeatures: {} - }; -} - const SUPPORTED_FEATURES = [ 'name', 'data', @@ -59,15 +25,6 @@ const SUPPORTED_FEATURES = [ 'refs', 'store' ]; -const isFeatureSupported = (v) => SUPPORTED_FEATURES.includes(v); - -const INFO_FEATURES_SUPPORTED = `Supported features: ${JSON.stringify(SUPPORTED_FEATURES)}`; - -function getUnsupporteFeaturesString(arr) { - return `Features [${utils.printArray(arr)}] in ` + - 'options.features are not supported by the SvelteV2 Parser. ' + - INFO_FEATURES_SUPPORTED; -} const EVENT_EMIT_RE = /\bfire\s*\(\s*((?:'[^']*')|(?:"[^"]*")|(?:`[^`]*`))/; @@ -117,7 +74,7 @@ class Parser extends EventEmitter { this.componentName = options.componentName; this.template = options.source.template; this.filename = options.filename; - this.eventsEmmited = options.eventsEmmited; + this.eventsEmitted = options.eventsEmitted; this.defaultMethodVisibility = options.defaultMethodVisibility; this.defaultActionVisibility = options.defaultActionVisibility; this.identifiers = options.identifiers; @@ -131,7 +88,7 @@ class Parser extends EventEmitter { defaultActionVisibility: utils.DEFAULT_VISIBILITY, features: [...SUPPORTED_FEATURES], componentName: null, - eventsEmmited: {}, + eventsEmitted: {}, identifiers: {}, imports: {}, ast: getAstDefaultOptions(), @@ -153,7 +110,7 @@ class Parser extends EventEmitter { Parser.normalizeOptions(options); // Check the presence and basic format of multiple options - for (const key of ['eventsEmmited', 'identifiers', 'imports']) { + for (const key of ['eventsEmitted', 'identifiers', 'imports']) { const hasKey = (key in options); if (!hasKey) { @@ -175,26 +132,7 @@ class Parser extends EventEmitter { }); } - if ('features' in options) { - if (!Array.isArray(options.features)) { - throw new TypeError('options.features must be an array'); - } - - if (options.features.length === 0) { - throw new Error( - 'options.features must contain at least one feature. ' + - INFO_FEATURES_SUPPORTED - ); - } - - if (!options.features.every(isFeatureSupported)) { - const notSupported = options.features.filter( - (iv) => !isFeatureSupported(iv) - ); - - throw new Error(getUnsupporteFeaturesString(notSupported)); - } - } + validateFeatures(options, SUPPORTED_FEATURES); } static getEventName(feature) { @@ -276,13 +214,13 @@ class Parser extends EventEmitter { keywords: entry.keywords }; - if (hasOwnProperty(this.eventsEmmited, event.name)) { - const emitedEvent = this.eventsEmmited[event.name]; + if (hasOwnProperty(this.eventsEmitted, event.name)) { + const emitedEvent = this.eventsEmitted[event.name]; if (emitedEvent.parent) { event.visibility = 'public'; - this.eventsEmmited[event.name] = event; + this.eventsEmitted[event.name] = event; this.parseKeywords(entry.keywords, event); this.emit('event', event); @@ -290,7 +228,7 @@ class Parser extends EventEmitter { // This event already defined } } else { - this.eventsEmmited[event.name] = event; + this.eventsEmitted[event.name] = event; this.parseKeywords(entry.keywords, event); this.emit('event', event); @@ -617,15 +555,15 @@ class Parser extends EventEmitter { if (!event.name) { event.name = '****unhandled-event-name****'; } else { - if (hasOwnProperty(this.eventsEmmited, event.name)) { - const emitedEvent = this.eventsEmmited[event.name]; + if (hasOwnProperty(this.eventsEmitted, event.name)) { + const emitedEvent = this.eventsEmitted[event.name]; if (emitedEvent.visibility === 'public') { continue; } } - this.eventsEmmited[event.name] = event; + this.eventsEmitted[event.name] = event; } this.parseKeywords(event.keywords, event); @@ -769,8 +707,8 @@ class Parser extends EventEmitter { event.description = comment.description || ''; event.keywords = comment.keywords; - if (!hasOwnProperty(this.eventsEmmited, event.name)) { - this.eventsEmmited[event.name] = event; + if (!hasOwnProperty(this.eventsEmitted, event.name)) { + this.eventsEmitted[event.name] = event; this.parseKeywords(comment.keywords, event); this.emit('event', event); diff --git a/lib/v3/parser.js b/lib/v3/parser.js index 7f423b7..c435a64 100644 --- a/lib/v3/parser.js +++ b/lib/v3/parser.js @@ -7,10 +7,9 @@ const HtmlParser = require('htmlparser2-svelte').Parser; const utils = require('./../utils'); const jsdoc = require('./../jsdoc'); +const { validateFeatures, getAstDefaultOptions } = require('../options'); -const hasOwnProperty = (obj, prop) => Object.prototype.hasOwnProperty.call(obj, prop); - -const DEFAULT_OPTIONS = {}; +const hasOwnProperty = utils.hasOwnProperty; const SUPPORTED_FEATURES = [ 'name', @@ -24,7 +23,6 @@ const SUPPORTED_FEATURES = [ 'slots', 'refs' ]; - const SCOPE_DEFAULT = 'default'; const SCOPE_STATIC = 'static'; const SCOPE_MARKUP = 'markup'; @@ -43,21 +41,21 @@ class Parser extends EventEmitter { constructor(options) { super(); - this._options = Object.assign({}, DEFAULT_OPTIONS, options); + Parser.validateOptions(options); this.structure = options.structure; - this.features = options.features || SUPPORTED_FEATURES; + this.features = options.features; this.includeSourceLocations = options.includeSourceLocations; - this.componentName = null; + this.componentName = options.componentName; this.filename = options.filename; - this.eventsEmitted = {}; + this.eventsEmitted = options.eventsEmitted; this.defaultMethodVisibility = options.defaultMethodVisibility; this.defaultActionVisibility = options.defaultActionVisibility; - this.identifiers = {}; - this.imports = {}; + this.identifiers = options.identifiers; + this.imports = options.imports; - this.dispatcherConstructorNames = []; - this.dispatcherNames = []; + this.dispatcherConstructorNames = options.dispatcherConstructorNames; + this.dispatcherNames = options.dispatcherNames; } walk() { @@ -90,6 +88,53 @@ class Parser extends EventEmitter { this.emit('end'); } + static getDefaultOptions() { + return { + includeSourceLocations: true, + defaultMethodVisibility: utils.DEFAULT_VISIBILITY, + defaultActionVisibility: utils.DEFAULT_VISIBILITY, + features: [...SUPPORTED_FEATURES], + componentName: null, + eventsEmitted: {}, + identifiers: {}, + imports: {}, + dispatcherConstructorNames: [], + dispatcherNames: [], + }; + } + + static normalizeOptions(options) { + const defaults = Parser.getDefaultOptions(); + + Object.keys(defaults).forEach((optionKey) => { + // If the key was not set by the user, apply default value. + if (!(optionKey in options)) { + options[optionKey] = defaults[optionKey]; + } + }); + } + + static validateOptions(options) { + Parser.normalizeOptions(options); + + // Check the presence and basic format of multiple options + for (const key of ['eventsEmitted', 'identifiers', 'imports']) { + const hasKey = (key in options); + + if (!hasKey) { + throw new Error(`options.${key} is required`); + } + + const hasCorrectType = typeof options[key] === 'object'; + + if (!hasCorrectType) { + throw new TypeError(`options.${key} must be of type 'object'`); + } + } + + validateFeatures(options, SUPPORTED_FEATURES); + } + static getEventName(feature) { return feature.endsWith('s') ? feature.substring(0, feature.length - 1) @@ -531,23 +576,10 @@ class Parser extends EventEmitter { }); } - getAstParsingOptions() { - return { - tokens: true, - loc: true, - range: true, - ecmaVersion: 9, - sourceType: 'module', - comment: true, - ecmaFeatures: { - } - }; - } - parseScriptBlock(scriptBlock) { const ast = espree.parse( scriptBlock.content, - this.getAstParsingOptions() + getAstDefaultOptions() ); const sourceCode = new eslint.SourceCode({ @@ -583,7 +615,7 @@ class Parser extends EventEmitter { const ast = espree.parse( expression, - this.getAstParsingOptions() + getAstDefaultOptions() ); const sourceCode = new eslint.SourceCode({ diff --git a/test/integration/parse/parse.spec.js b/test/integration/parse/parse.spec.js index 46e67c2..a97d3ef 100644 --- a/test/integration/parse/parse.spec.js +++ b/test/integration/parse/parse.spec.js @@ -3,6 +3,11 @@ const chai = require('chai'); const expect = chai.expect; const parser = require('./../../../index'); +const { ParserError } = require('../../../lib/options'); +const { AssertionError } = require('chai'); +const { + SUPPORTED_FEATURES: V3_SUPPORTED_FEATURES +} = require('../../../lib/v3/parser'); describe('parse - Integration', () => { it('should correctly auto-detect svelte V2 component', (done) => { @@ -23,7 +28,7 @@ describe('parse - Integration', () => { parser.parse({ filename: path.resolve(__dirname, 'basicV3.svelte'), }).then((doc) => { - expect(doc, 'Document should be provided').to.exist; + expect(doc, 'Document should exist').to.exist; // v3-parser converts component name to PascalCase expect(doc.name).to.equal('BasicV3'); @@ -32,4 +37,21 @@ describe('parse - Integration', () => { done(e); }); }); + + it('should throw when passed unsupported features', (done) => { + parser.parse({ + version: 3, + filename: path.resolve(__dirname, 'basicV3.svelte'), + features: ['data', 'unsupported'], + }).then(() => { + done(new AssertionError( + 'parser.parse should throw ParserError.FeaturesNotSupported' + )); + }).catch(e => { + expect(e.message).is.equal(ParserError.FeaturesNotSupported( + ['unsupported'], V3_SUPPORTED_FEATURES + )); + done(); + }); + }); }); diff --git a/test/unit/options/options.spec.js b/test/unit/options/options.spec.js index 76d2f0a..962b9ad 100644 --- a/test/unit/options/options.spec.js +++ b/test/unit/options/options.spec.js @@ -2,8 +2,10 @@ const expect = require('chai').expect; const { validate, + validateFeatures, retrieveFileOptions, - ErrorMessage + OptionsError, + ParserError } = require('../../../lib/options'); const baseOptions = { filename: 'empty.svelte' }; @@ -12,20 +14,20 @@ describe('Options Module', () => { describe('options.validate', () => { describe('Should throw when', () => { it('options object is missing', () => { - expect(() => validate()).to.throw(ErrorMessage.OptionsRequired); + expect(() => validate()).to.throw(OptionsError.OptionsRequired); }); it('input is missing, not a string, or an empty filename', () => { - expect(() => validate({})).to.throw(ErrorMessage.InputRequired); - expect(() => validate({ filename: {} })).to.throw(ErrorMessage.InputRequired); - expect(() => validate({ filename: '' })).to.throw(ErrorMessage.InputRequired); - expect(() => validate({ fileContent: {} })).to.throw(ErrorMessage.InputRequired); + expect(() => validate({})).to.throw(OptionsError.InputRequired); + expect(() => validate({ filename: {} })).to.throw(OptionsError.InputRequired); + expect(() => validate({ filename: '' })).to.throw(OptionsError.InputRequired); + expect(() => validate({ fileContent: {} })).to.throw(OptionsError.InputRequired); }); it('encoding is not a string', () => { const options = { ...baseOptions, encoding: true }; - expect(() => validate(options)).to.throw(ErrorMessage.EncodingFormat); + expect(() => validate(options)).to.throw(OptionsError.EncodingFormat); }); it('encoding is not supported', () => { @@ -33,7 +35,7 @@ describe('Options Module', () => { const options = { ...baseOptions, encoding: unsupported }; expect(() => validate(options)).to.throw( - ErrorMessage.EncodingNotSupported(unsupported) + OptionsError.EncodingNotSupported(unsupported) ); }); @@ -43,7 +45,7 @@ describe('Options Module', () => { const options = { ...baseOptions, ignoredVisibilities: mixed }; expect(() => validate(options)).to.throw( - ErrorMessage.IgnoredVisibilitiesNotSupported([unsupported]) + OptionsError.IgnoredVisibilitiesNotSupported([unsupported]) ); }); @@ -53,7 +55,7 @@ describe('Options Module', () => { const options = { ...baseOptions, ignoredVisibilities: mixed }; expect(() => validate(options)).to.throw( - ErrorMessage.IgnoredVisibilitiesNotSupported([unsupported]) + OptionsError.IgnoredVisibilitiesNotSupported([unsupported]) ); }); }); @@ -88,6 +90,74 @@ describe('Options Module', () => { }); }); + describe('options.validateFeatures', () => { + describe('Should pass when', () => { + it('only supported features are present', () => { + const supported = ['something', 'else']; + + const single = ['something']; + const options1 = { features: single }; + + expect(() => validateFeatures(options1, supported)).to.not.throw(); + + const all = ['else', 'something']; + const options2 = { features: all }; + + expect(() => validateFeatures(options2, supported)).to.not.throw(); + }); + }); + + describe('Should throw when', () => { + it('features is not an array', () => { + expect(() => validateFeatures({ features: {} }, [])) + .to.throw(ParserError.FeaturesFormat); + + expect(() => validateFeatures({ features: true }, [])) + .to.throw(ParserError.FeaturesFormat); + + expect(() => validateFeatures({ features: 'something' }, [])) + .to.throw(ParserError.FeaturesFormat); + }); + + it('features is an empty array', () => { + const supported = ['something', 'else']; + + expect(() => validateFeatures({ features: [] }, supported)) + .to.throw(ParserError.FeaturesEmpty(supported)); + }); + + it('one or more features are not supported', () => { + const supported = ['something', 'else']; + + const notSupported1 = ['other']; + const options1 = { features: notSupported1 }; + + expect(() => validateFeatures(options1, supported)).to.throw( + ParserError.FeaturesNotSupported(notSupported1, supported) + ); + + const notSupported2 = ['other', 'bad', 'trash']; + const options2 = { features: notSupported2 }; + + expect(() => validateFeatures(options2, supported)).to.throw( + ParserError.FeaturesNotSupported(notSupported2, supported) + ); + }); + + it('some features are not supported', () => { + const supported = ['something', 'else', 'stuff']; + const notSupported = ['other', 'thing']; + + const mixed = ['stuff', ...notSupported, 'something']; + const options2 = { features: mixed }; + + expect(() => validateFeatures(options2, supported)).to.throw( + ParserError.FeaturesNotSupported(notSupported, supported) + ); + }); + }); + }); + describe('options.retrieveFileOptions', () => { it('Should return all file-related keys from options', () => { expect(retrieveFileOptions(baseOptions)).to.have.keys( From 0830d0a1f537c8d75e411d086f972d6cf8bcc91a Mon Sep 17 00:00:00 2001 From: Pierre-Antoine Rousseau Date: Mon, 7 Dec 2020 17:34:11 -0500 Subject: [PATCH 3/4] test(options-v2): Add integration test for v2 features validation - fix(test): add second catch block in integration/parse --- test/integration/parse/parse.spec.js | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/test/integration/parse/parse.spec.js b/test/integration/parse/parse.spec.js index a97d3ef..274dab7 100644 --- a/test/integration/parse/parse.spec.js +++ b/test/integration/parse/parse.spec.js @@ -8,6 +8,9 @@ const { AssertionError } = require('chai'); const { SUPPORTED_FEATURES: V3_SUPPORTED_FEATURES } = require('../../../lib/v3/parser'); +const { + SUPPORTED_FEATURES: V2_SUPPORTED_FEATURES +} = require('../../../lib/parser'); describe('parse - Integration', () => { it('should correctly auto-detect svelte V2 component', (done) => { @@ -38,7 +41,26 @@ describe('parse - Integration', () => { }); }); - it('should throw when passed unsupported features', (done) => { + it('should throw when svelte V2 parser receives unsupported features', (done) => { + parser.parse({ + version: 2, + filename: path.resolve(__dirname, 'basicV2.svelte'), + features: ['data', 'unsupported'], + }).then(() => { + done(new AssertionError( + 'parser.parse should throw ParserError.FeaturesNotSupported' + )); + }).catch(e => { + expect(e.message).is.equal(ParserError.FeaturesNotSupported( + ['unsupported'], V2_SUPPORTED_FEATURES + )); + done(); + }).catch(e => { + done(e); + }); + }); + + it('should throw when svelte V3 parser receives unsupported features', (done) => { parser.parse({ version: 3, filename: path.resolve(__dirname, 'basicV3.svelte'), @@ -52,6 +74,8 @@ describe('parse - Integration', () => { ['unsupported'], V3_SUPPORTED_FEATURES )); done(); + }).catch(e => { + done(e); }); }); }); From 40d2a1a3ce604129f043cc3f5f6fa86f0ebcc710 Mon Sep 17 00:00:00 2001 From: Pierre-Antoine Rousseau Date: Tue, 8 Dec 2020 13:25:40 -0500 Subject: [PATCH 4/4] refactor(options): simplify validations made on internal properties - feat(options): validate includeSourceLocations - test(options): Add tests for includeSourceLocations - refactor(v2-parser): extract function for sourceCode generation - refactor: move normalizeOptions to lib/options --- lib/options.js | 31 +++++-- lib/parser.js | 138 +++++++++++++----------------- lib/v3/parser.js | 61 ++++--------- test/unit/options/options.spec.js | 16 ++++ 4 files changed, 114 insertions(+), 132 deletions(-) diff --git a/lib/options.js b/lib/options.js index 0122f0b..2731b56 100644 --- a/lib/options.js +++ b/lib/options.js @@ -2,6 +2,7 @@ const { isString, printArray, isVisibilitySupported, VISIBILITIES } = require('. /** * @typedef {import("../typings").SvelteParserOptions} SvelteParserOptions + * @typedef {import("../typings").JSVisibilityScope} JSVisibilityScope */ /** @type {BufferEncoding[]} */ @@ -43,12 +44,14 @@ const OptionsError = Object.freeze({ IgnoredVisibilitiesMissing: 'Internal Error: options.ignoredVisibilities is not set.', IgnoredVisibilitiesFormat: ERROR_VISIBILITIES_FORMAT + INFO_VISIBILITIES_SUPPORTED, IgnoredVisibilitiesNotSupported: (arr) => getUnsupportedVisibilitiesString(arr), + IncludeSourceLocationsMissing: 'Internal Error: options.includeSourceLocationsMissing is not set.', + IncludeSourceLocationsFormat: 'Expected options.includeSourceLocations to be a boolean.', }); /** @type {BufferEncoding} */ const DEFAULT_ENCODING = 'utf8'; -/** @type {SymbolVisibility[]} */ +/** @type {JSVisibilityScope[]} */ const DEFAULT_IGNORED_VISIBILITIES = ['protected', 'private']; /** @returns {SvelteParserOptions} */ @@ -56,6 +59,7 @@ function getDefaultOptions() { return { encoding: DEFAULT_ENCODING, ignoredVisibilities: [...DEFAULT_IGNORED_VISIBILITIES], + includeSourceLocations: false, }; } @@ -70,11 +74,10 @@ function retrieveFileOptions(options) { /** * Applies default values to options. - * @param {SvelteParserOptions} options + * @param {SvelteParserOptions} options object to normalize (mutated) + * @param {SvelteParserOptions} defaults default values to normalize 'options' */ -function normalize(options) { - const defaults = getDefaultOptions(); - +function normalize(options, defaults) { Object.keys(defaults).forEach((optionKey) => { /** * If the key was not set by the user, apply default value. @@ -97,7 +100,7 @@ function validate(options) { throw new Error(OptionsError.OptionsRequired); } - normalize(options); + normalize(options, getDefaultOptions()); const hasFilename = ('filename' in options) && @@ -122,7 +125,7 @@ function validate(options) { throw new Error(OptionsError.EncodingNotSupported(options.encoding)); } } else { - // Sanity check. At this point, 'encoding' should be set. + // Sanity check. At this point, 'encoding' must be set. throw new Error(OptionsError.EncodingMissing); } @@ -139,9 +142,18 @@ function validate(options) { throw new Error(OptionsError.IgnoredVisibilitiesNotSupported(notSupported)); } } else { - // Sanity check. At this point, 'ignoredVisibilities' should be set. + // Sanity check. At this point, 'ignoredVisibilities' must be set. throw new Error(OptionsError.IgnoredVisibilitiesMissing); } + + if ('includeSourceLocations' in options) { + if (typeof options.includeSourceLocations !== 'boolean') { + throw new TypeError(OptionsError.IncludeSourceLocationsFormat); + } + } else { + // Sanity check. At this point, 'includeSourceLocations' must be set. + throw new Error(OptionsError.IncludeSourceLocationsMissing); + } } const getSupportedFeaturesString = (supported) => `Supported features: ${printArray(supported)}`; @@ -170,7 +182,7 @@ const ParserError = { /** * - * @param {*} options + * @param {SvelteParserOptions} options * @param {string[]} supported * @throws if any validation fails for options.features */ @@ -232,6 +244,7 @@ function getAstDefaultOptions() { module.exports = { OptionsError, ParserError, + normalize, validate, validateFeatures, retrieveFileOptions, diff --git a/lib/parser.js b/lib/parser.js index 4473090..b578f93 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -5,7 +5,11 @@ const HtmlParser = require('htmlparser2-svelte').Parser; const path = require('path'); const utils = require('./utils'); const jsdoc = require('./jsdoc'); -const { validateFeatures, getAstDefaultOptions } = require('./options'); +const { + normalize: normalizeOptions, + validateFeatures, + getAstDefaultOptions +} = require('./options'); const hasOwnProperty = utils.hasOwnProperty; @@ -28,101 +32,83 @@ const SUPPORTED_FEATURES = [ const EVENT_EMIT_RE = /\bfire\s*\(\s*((?:'[^']*')|(?:"[^"]*")|(?:`[^`]*`))/; +function generateSourceCode(options) { + const generated = { + ast: null, + sourceCode: null, + scriptOffset: 0, + }; + + if (!(hasOwnProperty(options.source, 'script') && options.source.script)) { + return generated; + } + + try { + generated.ast = espree.parse( + options.source.script, + getAstDefaultOptions() + ); + + generated.sourceCode = new eslint.SourceCode({ + text: options.source.script, + ast: generated.ast + }); + } catch (e) { + const script = utils.escapeImportKeyword(options.source.script); + + generated.ast = espree.parse(script, getAstDefaultOptions()); + + generated.sourceCode = new eslint.SourceCode({ + text: script, + ast: generated.ast + }); + } + + generated.scriptOffset = options.source.scriptOffset || 0; + + return generated; +} + class Parser extends EventEmitter { constructor(options) { super(); Parser.validateOptions(options); + // External options + this.filename = options.filename; this.source = options.source; + this.template = options.source.template; this.features = options.features; + this.includeSourceLocations = options.includeSourceLocations; - if (hasOwnProperty(options.source, 'script') && options.source.script) { - this.scriptOffset = options.source.scriptOffset || 0; - - try { - this.ast = espree.parse(options.source.script, options.ast); - - this.sourceCode = new eslint.SourceCode({ - text: options.source.script, - ast: this.ast - }); - } catch (e) { - const script = utils.escapeImportKeyword(options.source.script); - - const newAstOptions = { - ...options.ast, - loc: true, - range: true, - comment: true - }; - - this.ast = espree.parse(script, newAstOptions); + // Internal Properties + this.defaultMethodVisibility = utils.DEFAULT_VISIBILITY; + this.defaultActionVisibility = utils.DEFAULT_VISIBILITY; + this.componentName = null; + this.eventsEmitted = {}; + this.identifiers = {}; + this.imports = {}; - this.sourceCode = new eslint.SourceCode({ - text: script, - ast: this.ast - }); - } - } else { - this.scriptOffset = 0; - this.ast = null; - this.sourceCode = null; - } + // Generated Espree AST + const { ast, sourceCode, scriptOffset } = generateSourceCode(options); - this.includeSourceLocations = options.includeSourceLocations; - this.componentName = options.componentName; - this.template = options.source.template; - this.filename = options.filename; - this.eventsEmitted = options.eventsEmitted; - this.defaultMethodVisibility = options.defaultMethodVisibility; - this.defaultActionVisibility = options.defaultActionVisibility; - this.identifiers = options.identifiers; - this.imports = options.imports; + this.ast = ast; + this.sourceCode = sourceCode; + this.scriptOffset = scriptOffset; } static getDefaultOptions() { return { includeSourceLocations: true, - defaultMethodVisibility: utils.DEFAULT_VISIBILITY, - defaultActionVisibility: utils.DEFAULT_VISIBILITY, - features: [...SUPPORTED_FEATURES], - componentName: null, - eventsEmitted: {}, - identifiers: {}, - imports: {}, - ast: getAstDefaultOptions(), + features: [...SUPPORTED_FEATURES] }; } - static normalizeOptions(options) { - const defaults = Parser.getDefaultOptions(); - - Object.keys(defaults).forEach((optionKey) => { - // If the key was not set by the user, apply default value. - if (!(optionKey in options)) { - options[optionKey] = defaults[optionKey]; - } - }); - } - static validateOptions(options) { - Parser.normalizeOptions(options); - - // Check the presence and basic format of multiple options - for (const key of ['eventsEmitted', 'identifiers', 'imports']) { - const hasKey = (key in options); - - if (!hasKey) { - throw new Error(`options.${key} is required`); - } - - const hasCorrectType = typeof options[key] === 'object'; + normalizeOptions(options, Parser.getDefaultOptions()); - if (!hasCorrectType) { - throw new TypeError(`options.${key} must be of type 'object'`); - } - } + validateFeatures(options, SUPPORTED_FEATURES); if ('source' in options) { ['script', 'scriptOffset'].forEach(key => { @@ -131,8 +117,6 @@ class Parser extends EventEmitter { } }); } - - validateFeatures(options, SUPPORTED_FEATURES); } static getEventName(feature) { diff --git a/lib/v3/parser.js b/lib/v3/parser.js index c435a64..ddc3615 100644 --- a/lib/v3/parser.js +++ b/lib/v3/parser.js @@ -7,7 +7,11 @@ const HtmlParser = require('htmlparser2-svelte').Parser; const utils = require('./../utils'); const jsdoc = require('./../jsdoc'); -const { validateFeatures, getAstDefaultOptions } = require('../options'); +const { + normalize: normalizeOptions, + validateFeatures, + getAstDefaultOptions +} = require('../options'); const hasOwnProperty = utils.hasOwnProperty; @@ -43,19 +47,18 @@ class Parser extends EventEmitter { Parser.validateOptions(options); + // External options + this.filename = options.filename; this.structure = options.structure; this.features = options.features; this.includeSourceLocations = options.includeSourceLocations; - this.componentName = options.componentName; - this.filename = options.filename; - this.eventsEmitted = options.eventsEmitted; - this.defaultMethodVisibility = options.defaultMethodVisibility; - this.defaultActionVisibility = options.defaultActionVisibility; - this.identifiers = options.identifiers; - this.imports = options.imports; - - this.dispatcherConstructorNames = options.dispatcherConstructorNames; - this.dispatcherNames = options.dispatcherNames; + + // Internal properties + this.componentName = null; + this.eventsEmitted = {}; + this.imports = {}; + this.dispatcherConstructorNames = []; + this.dispatcherNames = []; } walk() { @@ -91,46 +94,12 @@ class Parser extends EventEmitter { static getDefaultOptions() { return { includeSourceLocations: true, - defaultMethodVisibility: utils.DEFAULT_VISIBILITY, - defaultActionVisibility: utils.DEFAULT_VISIBILITY, features: [...SUPPORTED_FEATURES], - componentName: null, - eventsEmitted: {}, - identifiers: {}, - imports: {}, - dispatcherConstructorNames: [], - dispatcherNames: [], }; } - static normalizeOptions(options) { - const defaults = Parser.getDefaultOptions(); - - Object.keys(defaults).forEach((optionKey) => { - // If the key was not set by the user, apply default value. - if (!(optionKey in options)) { - options[optionKey] = defaults[optionKey]; - } - }); - } - static validateOptions(options) { - Parser.normalizeOptions(options); - - // Check the presence and basic format of multiple options - for (const key of ['eventsEmitted', 'identifiers', 'imports']) { - const hasKey = (key in options); - - if (!hasKey) { - throw new Error(`options.${key} is required`); - } - - const hasCorrectType = typeof options[key] === 'object'; - - if (!hasCorrectType) { - throw new TypeError(`options.${key} must be of type 'object'`); - } - } + normalizeOptions(options, Parser.getDefaultOptions()); validateFeatures(options, SUPPORTED_FEATURES); } diff --git a/test/unit/options/options.spec.js b/test/unit/options/options.spec.js index 962b9ad..ce00000 100644 --- a/test/unit/options/options.spec.js +++ b/test/unit/options/options.spec.js @@ -58,6 +58,14 @@ describe('Options Module', () => { OptionsError.IgnoredVisibilitiesNotSupported([unsupported]) ); }); + + it('includeSourceLocations is not a boolean', () => { + const options = { ...baseOptions, includeSourceLocations: 'true' }; + + expect(() => validate(options)).to.throw( + OptionsError.IncludeSourceLocationsFormat + ); + }); }); describe('Should pass when', () => { @@ -87,6 +95,14 @@ describe('Options Module', () => { expect(() => validate(options)).to.not.throw(); }); + + it('includeSourceLocations is a boolean', () => { + const options1 = { ...baseOptions, includeSourceLocations: false }; + const options2 = { ...baseOptions, includeSourceLocations: true }; + + expect(() => validate(options1)).to.not.throw(); + expect(() => validate(options2)).to.not.throw(); + }); }); });