From 086b7f10027e41e6119756ea58f627b7556977b3 Mon Sep 17 00:00:00 2001 From: Joe Hildebrand Date: Thu, 13 Oct 2022 08:05:02 -0600 Subject: [PATCH 1/3] Move glob munging all to one place, and go back to previous behavior of parseString --- src/index.test.ts | 5 +++++ src/index.ts | 16 ++++++---------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/index.test.ts b/src/index.test.ts index fe91d69..ab433c0 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -122,4 +122,9 @@ describe('parseString', () => { const cfg = editorconfig.parseString('root: ') cfg.should.eql([[null, {}]]) }) + + it('handles backslashes in glob', () => { + const cfg = editorconfig.parseString('[a\\\\b]') + cfg.should.eql([[null, {}], ['a\\\\b', {}]]) + }) }) diff --git a/src/index.ts b/src/index.ts index eec968b..7c4f4d2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -10,6 +10,7 @@ import { parse_to_uint32array, TokenTypes } from '@one-ini/wasm' import pkg from '../package.json' const escapedSep = new RegExp(path.sep.replace(/\\/g, '\\\\'), 'g') +const matchOptions = { matchBase: true, dot: true, noext: true } // These are specified by the editorconfig script /* eslint-disable @typescript-eslint/naming-convention */ @@ -78,9 +79,7 @@ export function parseBuffer(data: Buffer): ParseStringResult { case TokenTypes.Section: { cur = {} res.push([ - data - .toString('utf8', parsed[i+1], parsed[i+2]) - .replace(/\\\\/g, '\\\\\\\\'), + data.toString('utf8', parsed[i+1], parsed[i+2]), cur, ]) break @@ -103,12 +102,6 @@ export function parseString(data: string): ParseStringResult { return parseBuffer(Buffer.from(data)) } -function fnmatch(filepath: string, glob: string): boolean { - const matchOptions = { matchBase: true, dot: true, noext: true } - glob = glob.replace(/\*\*/g, '{*,**/**/**}') - return minimatch(filepath, glob, matchOptions) -} - function getConfigFileNames(filepath: string, options: ParseOptions): string[] { const paths = [] do { @@ -175,6 +168,9 @@ function buildFullGlob(pathPrefix: string, glob: string): string { default: break } + glob = glob.replace(/\\\\/g, '\\\\\\\\') + glob = glob.replace(/\*\*/g, '{*,**/**/**}') + // NOT path.join. Must stay in forward slashes. return `${pathPrefix}/${glob}` } @@ -236,7 +232,7 @@ function parseFromConfigs( return } const fullGlob = buildFullGlob(pathPrefix, glob) - if (!fnmatch(filepath, fullGlob)) { + if (!minimatch(filepath, fullGlob, matchOptions)) { return } if (options.files) { From 7238e3d14f7c20b48a4e3c613f4aa0b54e694b13 Mon Sep 17 00:00:00 2001 From: Joe Hildebrand Date: Fri, 14 Oct 2022 15:12:04 -0600 Subject: [PATCH 2/3] Add caching support for longer-running processes, so that the same .editorconfig file isn't read, parsed, and processed many times. --- README.md | 78 ++++++--- src/cli.ts | 42 ++++- src/index.test.ts | 50 ++++++ src/index.ts | 438 +++++++++++++++++++++++++++++++++------------- 4 files changed, 450 insertions(+), 158 deletions(-) diff --git a/README.md b/README.md index 3075eb5..3e81fcd 100644 --- a/README.md +++ b/README.md @@ -24,22 +24,56 @@ $ npm install -g editorconfig ## Usage -### in Node.js: - -#### parse(filePath[, options]) +### Options -options is an object with the following defaults: +Most of the API takes an `options` object, which has the following defaults: ```js { config: '.editorconfig', version: pkg.version, root: '/', - files: undefined + files: undefined, + cache: undefined, }; ``` -Search for `.editorconfig` starting from the current directory to the root directory. +
+
config
+
The name of the config file to look for in the current and every parent + directory.
+ +
version
+
Which editorconfig spec version to use. Earlier versions had different + defaults.
+ +
root
+
What directory to stop processing in, even if we haven't found a file + containing root=true. Defaults to the root of the filesystem containing + `process.cwd()`.
+ +
files
+
Pass in an empty array, which will be filled with one object for each + config file processed. The objects will have the shape + `{filename: "[DIRECTORY]/.editorconfig", glob: "*"}`
+ +
cache
+
If you are going to process more than one file in the same project, pass + in a cache object. It must have `get(string): object|undefined` and + `set(string, object)` methods, like a JavaScript Map. A long-running + process might want to consider that this cache might grow over time, + and that the config files might change over time. However, we leave any + complexity of that nature to the caller, since there are so many different + approaches that might be taken based on latency, memory, and CPU trade-offs.
+
+ +### in Node.js: + +#### parse(filePath[, options]) + +Search for `.editorconfig` files starting from the current directory to the +root directory. Combine all of the sections whose section names match +filePath into a single object. Example: @@ -69,33 +103,25 @@ const filePath = path.join(__dirname, 'sample.js'); */ ``` -When the `files` option is an array, it will be filled with objects that -describe which .editorcofig files and glob section names contributed to the -returned configuration. - #### parseSync(filePath[, options]) Synchronous version of `editorconfig.parse()`. -#### parseString(fileContent) +#### parseBuffer(fileContent) -The `parse()` function above uses `parseString()` under the hood. If you have your file contents -just pass it to `parseString()` and it'll return the same results as `parse()`. +The `parse()` function above uses `parseBuffer()` under the hood. If you have +the contents of a config file, and want to see what is being processed for +just that file rather than the full directory hierarchy, this might be useful. -#### parseFromFiles(filePath, configs[, options]) +#### parseString(fileContent) -options is an object with the following defaults: +This is a thin wrapper around `parseBuffer()` for backward-compatibility. +Prefer `parseBuffer()` to avoid an unnecessary UTF8-to-UTF16-to-UTF8 +conversion. Deprecated. -```js -{ - config: '.editorconfig', - version: pkg.version, - root: '/', - files: undefined -}; -``` +#### parseFromFiles(filePath, configs[, options]) -Specify the `.editorconfig`. +Low-level interface, which exists only for backward-compatibility. Deprecated. Example: @@ -115,7 +141,7 @@ const configs = [ const filePath = path.join(__dirname, '/sample.js'); (async () => { - console.log(await editorconfig.parseFromFiles(filePath, configs)) + console.log(await editorconfig.parseFromFiles(filePath, Promise.resolve(configs))) })(); /* { @@ -132,7 +158,7 @@ const filePath = path.join(__dirname, '/sample.js'); #### parseFromFilesSync(filePath, configs[, options]) -Synchronous version of `editorconfig.parseFromFiles()`. +Synchronous version of `editorconfig.parseFromFiles()`. Deprecated. ### in Command Line diff --git a/src/cli.ts b/src/cli.ts index 52cd9ee..0475b94 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -4,11 +4,27 @@ import * as editorconfig from './' import pkg from '../package.json' +/** + * Default output routine, goes to stdout. + * + * @param s String to output + */ function writeStdOut(s: string): void { process.stdout.write(s) } -export default function cli( +/** + * Command line interface for editorconfig. Pulled out into a separate module + * to make it easier to test. + * + * @param args Usually process.argv. Note that the first two parameters are + * usually 'node' and 'editorconfig' + * @param testing If testing, you may pass in a Commander OutputConfiguration + * so that you can capture stdout and stderror. If `testing` is provided, + * this routine will throw an error instead of calling `process.exit`. + * @returns An array of combined properties, one for each file argument. + */ +export default async function cli( args: string[], testing?: OutputConfiguration ): Promise { @@ -42,17 +58,27 @@ export default function cli( const files = program.args const opts = program.opts() + const cache = new Map() const visited = opts.files ? files.map(() => []) : undefined - return Promise.all( - files.map((filePath, i) => editorconfig.parse(filePath, { - config: opts.f as string, - version: opts.b as string, - files: visited ? visited[i] : undefined, - })) - ).then((parsed) => { + // Process sequentially so caching works + async function processAll(): Promise { + const p = [] + let i = 0 + for (const filePath of files) { + p.push(await editorconfig.parse(filePath, { + config: opts.f as string, + version: opts.b as string, + files: visited ? visited[i++] : undefined, + cache, + })) + } + return p + } + + return await processAll().then((parsed) => { const header = parsed.length > 1 parsed.forEach((props, i) => { if (header) { diff --git a/src/index.test.ts b/src/index.test.ts index ab433c0..056deec 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -33,6 +33,24 @@ describe('parse', () => { visited[0].glob.should.eql('*') visited[0].fileName.should.endWith('.editorconfig') }) + + it('caches', async () => { + const cache = new Map() + const cfg = await editorconfig.parse(target, {cache}) + cfg.should.eql(expected) + cache.size.should.be.eql(1) + await editorconfig.parse(target, {cache}) + cache.size.should.be.eql(1) + }) + + it('caches sync', () => { + const cache = new Map() + const cfg = editorconfig.parseSync(target, {cache}) + cfg.should.eql(expected) + cache.size.should.be.eql(1) + editorconfig.parseSync(target, {cache}) + cache.size.should.be.eql(1) + }) }) describe('parseFromFiles', () => { @@ -55,6 +73,10 @@ describe('parseFromFiles', () => { contents: fs.readFileSync(configPath), }) const target = path.join(__dirname, '/app.js') + const configs2 = [ + { name: 'early', contents: Buffer.alloc(0) }, + configs[0], + ] it('async', async () => { const cfg: editorconfig.Props = @@ -75,6 +97,34 @@ describe('parseFromFiles', () => { cfg.should.eql({ foo: 'null' }) }) + it('caches async', async () => { + const cache = new Map() + const cfg = await editorconfig.parseFromFiles( + target, Promise.resolve(configs2), {cache} + ) + cfg.should.eql(expected) + cache.size.should.be.eql(2) + const cfg2 = await editorconfig.parseFromFiles( + target, Promise.resolve(configs2), {cache} + ) + cfg2.should.eql(expected) + cache.size.should.be.eql(2) + }) + + it('caches sync', () => { + const cache = new Map() + const cfg = editorconfig.parseFromFilesSync( + target, configs2, {cache} + ) + cfg.should.eql(expected) + cache.size.should.be.eql(2) + const cfg2 = editorconfig.parseFromFilesSync( + target, configs2, {cache} + ) + cfg2.should.eql(expected) + cache.size.should.be.eql(2) + }) + it('handles minimatch escapables', () => { // Note that this `#` does not actually test the /^#/ escaping logic, // because this path will go through a `path.dirname` before that happens. diff --git a/src/index.ts b/src/index.ts index 7c4f4d2..97118c4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -35,9 +35,13 @@ export interface ECFile { contents?: Buffer } -export interface FileConfig { +type SectionGlob = minimatch.Minimatch | null +type GlobbedProps = [SectionName, Props, SectionGlob][] + +export interface ProcessedFileConfig { + root: boolean name: string - contents: ParseStringResult + config: GlobbedProps } export interface Visited { @@ -45,11 +49,17 @@ export interface Visited { glob: string } +export interface Cache { + get(path: string): ProcessedFileConfig | undefined + set(path: string, config: ProcessedFileConfig): this +} + export interface ParseOptions { config?: string version?: string root?: string files?: Visited[] + cache?: Cache } // These are specified by the editorconfig script @@ -68,6 +78,13 @@ export type SectionName = string | null export interface SectionBody { [key: string]: string } export type ParseStringResult = [SectionName, SectionBody][] +/** + * Parse a buffer using the faster one-ini WASM approach into something + * relatively easy to deal with in JS. + * + * @param data UTF8-encoded bytes. + * @returns Parsed contents. Will be truncated if there was a parse error. + */ export function parseBuffer(data: Buffer): ParseStringResult { const parsed = parse_to_uint32array(data) let cur: SectionBody = {} @@ -91,17 +108,33 @@ export function parseBuffer(data: Buffer): ParseStringResult { cur[key as string] = data.toString('utf8', parsed[i+1], parsed[i+2]) break } - default: + default: // Comments, etc. break } } return res } +/** + * Parses a string. If possible, you should always use ParseBuffer instead, + * since this function does a UTF16-to-UTF8 conversion first. + * + * @param data String to parse. + * @returns Parsed contents. Will be truncated if there was a parse error. + * @deprecated Use {@link ParseBuffer} instead. + */ export function parseString(data: string): ParseStringResult { return parseBuffer(Buffer.from(data)) } +/** + * Gets a list of *potential* filenames based on the path of the target + * filename. + * + * @param filepath File we are asking about. + * @param options Config file name and root directory + * @returns List of potential fully-qualified filenames that might have configs. + */ function getConfigFileNames(filepath: string, options: ParseOptions): string[] { const paths = [] do { @@ -111,6 +144,14 @@ function getConfigFileNames(filepath: string, options: ParseOptions): string[] { return paths } +/** + * Take a combined config for the target file, and tweak it slightly based on + * which editorconfig version's rules we are using. + * + * @param matches Combined config. + * @param version Editorconfig version to enforce. + * @returns The passed-in matches object, modified in place. + */ function processMatches(matches: Props, version: string): Props { // Set indent_size to 'tab' if indent_size is unspecified and // indent_style is set to 'tab'. @@ -145,19 +186,7 @@ function processMatches(matches: Props, version: string): Props { return matches } -function processOptions( - options: ParseOptions = {}, - filepath: string -): ParseOptions { - return { - config: options.config || '.editorconfig', - version: options.version || pkg.version, - root: path.resolve(options.root || path.parse(filepath).root), - files: options.files, - } -} - -function buildFullGlob(pathPrefix: string, glob: string): string { +function buildFullGlob(pathPrefix: string, glob: string): minimatch.Minimatch { switch (glob.indexOf('/')) { case -1: glob = '**/' + glob @@ -168,14 +197,26 @@ function buildFullGlob(pathPrefix: string, glob: string): string { default: break } + // braces_escaped_backslash2 + // backslash_not_on_windows glob = glob.replace(/\\\\/g, '\\\\\\\\') + // star_star_over_separator{1,3,5,6,9,15} glob = glob.replace(/\*\*/g, '{*,**/**/**}') // NOT path.join. Must stay in forward slashes. - return `${pathPrefix}/${glob}` + return new minimatch.Minimatch(`${pathPrefix}/${glob}`, matchOptions) } -function extendProps(props: Props, options: SectionBody): Props { +/** + * Normalize the properties read from a config file so that their key names + * are lowercased for the known properties, and their values are parsed into + * the correct JS types if possible. + * + * @param options + * @returns + */ +function normalizeProps(options: SectionBody): Props { + const props = {} for (const key in options) { if (options.hasOwnProperty(key)) { const value = options[key] @@ -199,65 +240,132 @@ function extendProps(props: Props, options: SectionBody): Props { return props } -function parseFromConfigs( - configs: FileConfig[], +/** + * Take the contents of a config file, and prepare it for use. If a cache is + * provided, the result will be stored there. As such, all of the higher-CPU + * work that is per-file should be done here. + * + * @param filepath The fully-qualified path of the file. + * @param contents The contents as read from that file. + * @param options Access to the cache. + * @returns Processed file with globs pre-computed. + */ +function processFileContents( filepath: string, + contents: Buffer, options: ParseOptions -): Props { - return processMatches( - configs - .reverse() - .reduce( - (matches: Props, file) => { - let pathPrefix = path.dirname(file.name) - - if (path.sep !== '/') { - // Windows-only - pathPrefix = pathPrefix.replace(escapedSep, '/') - } +): ProcessedFileConfig { + let pathPrefix = path.dirname(filepath) - // After Windows path backslash's are turned into slashes, so that - // the backslashes we add here aren't turned into forward slashes: - - // All of these characters are special to minimatch, but can be - // forced into path names on many file systems. Escape them. Note - // that these are in the order of the case statement in minimatch. - pathPrefix = pathPrefix.replace(/[?*+@!()|[\]{}]/g, '\\$&') - // I can't think of a way for this to happen in the filesystems I've - // seen (because of the path.dirname above), but let's be thorough. - pathPrefix = pathPrefix.replace(/^#/, '\\#') - - file.contents.forEach(([glob, options2]) => { - if (!glob) { - return - } - const fullGlob = buildFullGlob(pathPrefix, glob) - if (!minimatch(filepath, fullGlob, matchOptions)) { - return - } - if (options.files) { - options.files.push({fileName: file.name, glob}) - } - matches = extendProps(matches, options2) - }) - return matches - }, - {} - ), - options.version as string - ) + if (path.sep !== '/') { + // Windows-only + pathPrefix = pathPrefix.replace(escapedSep, '/') + } + + // After Windows path backslash's are turned into slashes, so that + // the backslashes we add here aren't turned into forward slashes: + + // All of these characters are special to minimatch, but can be + // forced into path names on many file systems. Escape them. Note + // that these are in the order of the case statement in minimatch. + pathPrefix = pathPrefix.replace(/[?*+@!()|[\]{}]/g, '\\$&') + // I can't think of a way for this to happen in the filesystems I've + // seen (because of the path.dirname above), but let's be thorough. + pathPrefix = pathPrefix.replace(/^#/, '\\#') + + const globbed: GlobbedProps = parseBuffer(contents).map(([name, body]) => [ + name, + normalizeProps(body), + name ? buildFullGlob(pathPrefix, name) : null, + ]) + + const res: ProcessedFileConfig = { + root: !!globbed[0][1].root, // globbed[0] is the global section + name: filepath, + config: globbed, + } + if (options.cache) { + options.cache.set(filepath, res) + } + return res } -function getConfigsForFiles(files: ECFile[]): FileConfig[] { - const configs: FileConfig[] = [] +/** + * Get a file from the cache, or read its contents from disk, process, and + * insert into the cache (if configured). + * + * @param filepath The fully-qualified path of the config file. + * @param options Access to the cache, if configured. + * @returns The processed file, or undefined if there was an error reading it. + */ +async function getConfig( + filepath: string, + options: ParseOptions +): Promise { + if (options.cache) { + const cached = options.cache.get(filepath) + if (cached) { + return cached + } + } + const contents = await new Promise(resolve => { + fs.readFile(filepath, (_, buf) => { + // Ignore errors. contents will be undefined + // Perhaps only file-not-found should be ignored? + resolve(buf) + }) + }) + if (!contents) { + return undefined + } + return processFileContents(filepath, contents, options) +} + +/** + * Get a file from the cache, or read its contents from disk, process, and + * insert into the cache (if configured). Synchronous. + * + * @param filepath The fully-qualified path of the config file. + * @param options Access to the cache, if configured. + * @returns The processed file, or undefined if there was an error reading it. + */ +function getConfigSync( + filepath: string, + options: ParseOptions +): ProcessedFileConfig|undefined { + if (options.cache) { + const cached = options.cache.get(filepath) + if (cached) { + return cached + } + } + try { + const contents = fs.readFileSync(filepath) + return processFileContents(filepath, contents, options) + } catch (_) { + // Ignore errors + return undefined + } +} + +/** + * Get all of the possibly-existing config files, stopping when one is marked + * root=true. + * + * @param files List of potential files + * @param options Access to cache if configured + * @returns List of processed configs for existing files + */ +async function getAllConfigs( + files: string[], + options: ParseOptions +): Promise { + const configs: ProcessedFileConfig[] = [] for (const file of files) { - if (file.contents) { - const contents = parseBuffer(file.contents) - configs.push({ - name: file.name, - contents, - }) - if ((contents[0][1].root || '').toLowerCase() === 'true') { + const config = await getConfig(file, options) + if (config) { + configs.push(config) + if (config.root) { break } } @@ -265,30 +373,38 @@ function getConfigsForFiles(files: ECFile[]): FileConfig[] { return configs } -async function readConfigFiles(filepaths: string[]): Promise { - return Promise.all( - filepaths.map((name) => new Promise((resolve) => { - fs.readFile(name, (_, contents) => { - // Ignore errors. contents will be undefined - resolve({ name, contents }) - }) - })) - ) -} - -function readConfigFilesSync(filepaths: string[]): ECFile[] { - const files: ECFile[] = [] - filepaths.forEach((name) => { - try { - const contents = fs.readFileSync(name) - files.push({ name, contents }) - } catch (_) { - // Ignored +/** + * Get all of the possibly-existing config files, stopping when one is marked + * root=true. Synchronous. + * + * @param files List of potential files + * @param options Access to cache if configured + * @returns List of processed configs for existing files + */ +function getAllConfigsSync( + files: string[], + options: ParseOptions +): ProcessedFileConfig[] { + const configs: ProcessedFileConfig[] = [] + for (const file of files) { + const config = getConfigSync(file, options) + if (config) { + configs.push(config) + if (config.root) { + break + } } - }) - return files + } + return configs } +/** + * Normalize the options passed in to the publicly-visible functions. + * + * @param filepath The name of the target file, relative to process.cwd(). + * @param options Potentially-incomplete options. + * @returns The fully-qualified target file name and the normalized options. + */ function opts(filepath: string, options: ParseOptions = {}): [ string, ParseOptions @@ -296,62 +412,136 @@ function opts(filepath: string, options: ParseOptions = {}): [ const resolvedFilePath = path.resolve(filepath) return [ resolvedFilePath, - processOptions(options, resolvedFilePath), + { + config: options.config || '.editorconfig', + version: options.version || pkg.version, + root: path.resolve(options.root || path.parse(resolvedFilePath).root), + files: options.files, + cache: options.cache, + }, ] } +/** + * Low-level interface, which exists only for backward-compatibility. + * Deprecated. + * + * @param filepath The name of the target file, relative to process.cwd(). + * @param files A promise for a list of objects describing the files. + * @param options All options + * @returns The properties found for filepath + * @deprecated + */ export async function parseFromFiles( filepath: string, files: Promise, options: ParseOptions = {} ): Promise { - const [resolvedFilePath, processedOptions] = opts(filepath, options) - return files.then(getConfigsForFiles) - .then((configs) => parseFromConfigs( - configs, - resolvedFilePath, - processedOptions - )) + return parseFromFilesSync(filepath, await files, options) } +/** + * Low-level interface, which exists only for backward-compatibility. + * Deprecated. + * + * @param filepath The name of the target file, relative to process.cwd(). + * @param files A list of objects describing the files. + * @param options All options + * @returns The properties found for filepath + * @deprecated + */ export function parseFromFilesSync( filepath: string, files: ECFile[], options: ParseOptions = {} ): Props { const [resolvedFilePath, processedOptions] = opts(filepath, options) - return parseFromConfigs( - getConfigsForFiles(files), - resolvedFilePath, - processedOptions - ) + const configs = [] + for (const ecf of files) { + if (ecf.contents) { + if (options.cache) { + const cfg = options.cache.get(ecf.name) + if (cfg) { + configs.push(cfg) + if (cfg.root) { + break + } + continue + } + } + const cfg2 = processFileContents(ecf.name, ecf.contents, processedOptions) + configs.push(cfg2) + if (cfg2.root) { + break + } + } + } + return combine(resolvedFilePath, configs, processedOptions) } +/** + * Combine the pre-parsed results of all matching config file sections, in + * order. + * + * @param filepath The target file path + * @param configs All of the found config files, up to the root + * @param options Adds to `options.files` if it exists + * @returns Combined properties + */ +function combine( + filepath: string, + configs: ProcessedFileConfig[], + options: ParseOptions +): Props { + const ret = configs.reverse().reduce((props: Props, processed) => { + for (const [name, body, glob] of processed.config) { + if (glob && glob.match(filepath)) { + Object.assign(props, body) + if (options.files) { + options.files.push({ + fileName: processed.name, + glob: name as string, + }) + } + } + } + return props + }, {}) + return processMatches(ret, options.version as string) +} + +/** + * Find all of the properties from matching sections in config files in the + * same directory or toward the root of the filesystem. + * + * @param filepath The target file name, relative to process.cwd(). + * @param options All options + * @returns Combined properties for the target file + */ export async function parse( - _filepath: string, - _options: ParseOptions = {} + filepath: string, + options: ParseOptions = {} ): Promise { - const [resolvedFilePath, processedOptions] = opts(_filepath, _options) + const [resolvedFilePath, processedOptions] = opts(filepath, options) const filepaths = getConfigFileNames(resolvedFilePath, processedOptions) - return readConfigFiles(filepaths) - .then(getConfigsForFiles) - .then((configs) => parseFromConfigs( - configs, - resolvedFilePath, - processedOptions - )) + const configs = await getAllConfigs(filepaths, processedOptions) + return combine(resolvedFilePath, configs, processedOptions) } +/** + * Find all of the properties from matching sections in config files in the + * same directory or toward the root of the filesystem. Synchronous. + * + * @param filepath The target file name, relative to process.cwd(). + * @param options All options + * @returns Combined properties for the target file + */ export function parseSync( - _filepath: string, - _options: ParseOptions = {} + filepath: string, + options: ParseOptions = {} ): Props { - const [resolvedFilePath, processedOptions] = opts(_filepath, _options) + const [resolvedFilePath, processedOptions] = opts(filepath, options) const filepaths = getConfigFileNames(resolvedFilePath, processedOptions) - const files = readConfigFilesSync(filepaths) - return parseFromConfigs( - getConfigsForFiles(files), - resolvedFilePath, - processedOptions - ) + const configs = getAllConfigsSync(filepaths, processedOptions) + return combine(resolvedFilePath, configs, processedOptions) } From 8a15406c0b80e0d6c397f83ea07a93dc99a3c976 Mon Sep 17 00:00:00 2001 From: Joe Hildebrand Date: Sat, 15 Oct 2022 09:27:11 -0600 Subject: [PATCH 3/3] Add negative caching --- README.md | 8 +++- src/index.test.ts | 8 ++-- src/index.ts | 110 ++++++++++++++++++++++++---------------------- 3 files changed, 68 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index 3e81fcd..faddcd9 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,13 @@ Most of the API takes an `options` object, which has the following defaults: process might want to consider that this cache might grow over time, and that the config files might change over time. However, we leave any complexity of that nature to the caller, since there are so many different - approaches that might be taken based on latency, memory, and CPU trade-offs. + approaches that might be taken based on latency, memory, and CPU trade-offs. + Note that some of the objects in the cache will be for files that did not + exist. Those objects will have a `notfound: true` property. All of the + objects will have a `name: string` property that contains the + fully-qualified file name of the config file and a `root: boolean` property + that describes if the config file had a `root=true` at the top. Any other + properties in the objects should be treated as opaque. ### in Node.js: diff --git a/src/index.test.ts b/src/index.test.ts index 056deec..53dabdc 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -38,18 +38,18 @@ describe('parse', () => { const cache = new Map() const cfg = await editorconfig.parse(target, {cache}) cfg.should.eql(expected) - cache.size.should.be.eql(1) + cache.size.should.be.eql(2) await editorconfig.parse(target, {cache}) - cache.size.should.be.eql(1) + cache.size.should.be.eql(2) }) it('caches sync', () => { const cache = new Map() const cfg = editorconfig.parseSync(target, {cache}) cfg.should.eql(expected) - cache.size.should.be.eql(1) + cache.size.should.be.eql(2) editorconfig.parseSync(target, {cache}) - cache.size.should.be.eql(1) + cache.size.should.be.eql(2) }) }) diff --git a/src/index.ts b/src/index.ts index 97118c4..4ed4874 100644 --- a/src/index.ts +++ b/src/index.ts @@ -42,6 +42,7 @@ export interface ProcessedFileConfig { root: boolean name: string config: GlobbedProps + notfound?: true } export interface Visited { @@ -252,37 +253,48 @@ function normalizeProps(options: SectionBody): Props { */ function processFileContents( filepath: string, - contents: Buffer, + contents: Buffer|undefined, options: ParseOptions ): ProcessedFileConfig { - let pathPrefix = path.dirname(filepath) + let res: ProcessedFileConfig + if (!contents) { + // Negative cache + res = { + root: false, + notfound: true, + name: filepath, + config: [[ null, {}, null ]], + } + } else { + let pathPrefix = path.dirname(filepath) - if (path.sep !== '/') { - // Windows-only - pathPrefix = pathPrefix.replace(escapedSep, '/') - } + if (path.sep !== '/') { + // Windows-only + pathPrefix = pathPrefix.replace(escapedSep, '/') + } - // After Windows path backslash's are turned into slashes, so that - // the backslashes we add here aren't turned into forward slashes: - - // All of these characters are special to minimatch, but can be - // forced into path names on many file systems. Escape them. Note - // that these are in the order of the case statement in minimatch. - pathPrefix = pathPrefix.replace(/[?*+@!()|[\]{}]/g, '\\$&') - // I can't think of a way for this to happen in the filesystems I've - // seen (because of the path.dirname above), but let's be thorough. - pathPrefix = pathPrefix.replace(/^#/, '\\#') - - const globbed: GlobbedProps = parseBuffer(contents).map(([name, body]) => [ - name, - normalizeProps(body), - name ? buildFullGlob(pathPrefix, name) : null, - ]) - - const res: ProcessedFileConfig = { - root: !!globbed[0][1].root, // globbed[0] is the global section - name: filepath, - config: globbed, + // After Windows path backslash's are turned into slashes, so that + // the backslashes we add here aren't turned into forward slashes: + + // All of these characters are special to minimatch, but can be + // forced into path names on many file systems. Escape them. Note + // that these are in the order of the case statement in minimatch. + pathPrefix = pathPrefix.replace(/[?*+@!()|[\]{}]/g, '\\$&') + // I can't think of a way for this to happen in the filesystems I've + // seen (because of the path.dirname above), but let's be thorough. + pathPrefix = pathPrefix.replace(/^#/, '\\#') + + const globbed: GlobbedProps = parseBuffer(contents).map(([name, body]) => [ + name, + normalizeProps(body), + name ? buildFullGlob(pathPrefix, name) : null, + ]) + + res = { + root: !!globbed[0][1].root, // globbed[0] is the global section + name: filepath, + config: globbed, + } } if (options.cache) { options.cache.set(filepath, res) @@ -301,7 +313,7 @@ function processFileContents( async function getConfig( filepath: string, options: ParseOptions -): Promise { +): Promise { if (options.cache) { const cached = options.cache.get(filepath) if (cached) { @@ -315,9 +327,6 @@ async function getConfig( resolve(buf) }) }) - if (!contents) { - return undefined - } return processFileContents(filepath, contents, options) } @@ -332,20 +341,21 @@ async function getConfig( function getConfigSync( filepath: string, options: ParseOptions -): ProcessedFileConfig|undefined { +): ProcessedFileConfig { if (options.cache) { const cached = options.cache.get(filepath) if (cached) { return cached } } + let contents: Buffer | undefined try { - const contents = fs.readFileSync(filepath) - return processFileContents(filepath, contents, options) + contents = fs.readFileSync(filepath) } catch (_) { // Ignore errors - return undefined + // Perhaps only file-not-found should be ignored } + return processFileContents(filepath, contents, options) } /** @@ -363,7 +373,7 @@ async function getAllConfigs( const configs: ProcessedFileConfig[] = [] for (const file of files) { const config = await getConfig(file, options) - if (config) { + if (!config.notfound) { configs.push(config) if (config.root) { break @@ -388,7 +398,7 @@ function getAllConfigsSync( const configs: ProcessedFileConfig[] = [] for (const file of files) { const config = getConfigSync(file, options) - if (config) { + if (!config.notfound) { configs.push(config) if (config.root) { break @@ -458,24 +468,18 @@ export function parseFromFilesSync( const [resolvedFilePath, processedOptions] = opts(filepath, options) const configs = [] for (const ecf of files) { - if (ecf.contents) { - if (options.cache) { - const cfg = options.cache.get(ecf.name) - if (cfg) { - configs.push(cfg) - if (cfg.root) { - break - } - continue - } - } - const cfg2 = processFileContents(ecf.name, ecf.contents, processedOptions) - configs.push(cfg2) - if (cfg2.root) { - break - } + let cfg: ProcessedFileConfig | undefined + if (!options.cache || !(cfg = options.cache.get(ecf.name))) { // Single "="! + cfg = processFileContents(ecf.name, ecf.contents, processedOptions) + } + if (!cfg.notfound) { + configs.push(cfg) + } + if (cfg.root) { + break } } + return combine(resolvedFilePath, configs, processedOptions) }