From de2ddcb4bf6ed55beebc16641ed8bab4424eb66e Mon Sep 17 00:00:00 2001 From: Jamie Mason Date: Wed, 3 Jul 2019 15:31:06 +0100 Subject: [PATCH] feat(rule): resolve paths relative to project root --- src/move-files.spec.ts | 97 +++++++++++++++++++++++------------------- src/move-files.ts | 21 ++++++--- 2 files changed, 68 insertions(+), 50 deletions(-) diff --git a/src/move-files.spec.ts b/src/move-files.spec.ts index 6ed8adf..a064561 100644 --- a/src/move-files.spec.ts +++ b/src/move-files.spec.ts @@ -1,5 +1,6 @@ import { existsSync, readFileSync } from 'fs'; import * as mock from 'mock-fs'; +import { resolve } from 'path'; import { ERROR_FLAT_DIRECTORY, ERROR_MULTIPLE_TARGETS } from './config'; import rule from './move-files'; import { ruleTester } from './test/rule-tester'; @@ -25,16 +26,16 @@ describe('when no files are provided', () => { }); describe('when renaming one file', () => { - const oldPath = `/fake/dir/old-name.js`; - const newPath = `/fake/dir/new-name.js`; + const oldPath = `./fake/dir/old-name.js`; + const newPath = `./fake/dir/new-name.js`; const fileContents = `import { dep } from './dep'; export const a = 1;`; const errors = [{ message: `${oldPath} has moved to ${newPath}` }]; - const options = [{ files: { [oldPath]: newPath } }]; + const options = [{ files: { [oldPath]: './new-name.js' } }]; describe('when file already has the new name', () => { it('is valid', () => { ruleTester.run('move-files', rule, { - valid: [{ code: '', filename: newPath, options }], + valid: [{ code: '', filename: resolve(newPath), options }], invalid: [] }); }); @@ -56,7 +57,7 @@ describe('when renaming one file', () => { { code: fileContents, errors, - filename: oldPath, + filename: resolve(oldPath), options } ] @@ -74,31 +75,31 @@ describe('when renaming one file', () => { valid: [ { code: `import { a } from './new-name';`, - filename: `/fake/dir/sibling.js` + filename: resolve(`./fake/dir/sibling.js`) }, { code: `import { a } from '../new-name';`, - filename: `/fake/dir/dir/child.js` + filename: resolve(`./fake/dir/dir/child.js`) }, { code: `import { a } from './fake/dir/new-name';`, - filename: `/parent.js` + filename: resolve(`./parent.js`) } ], invalid: [ { code: `import { a } from './old-name';`, - filename: `/fake/dir/sibling.js`, + filename: resolve(`./fake/dir/sibling.js`), output: `import { a } from './new-name';` }, { code: `import { a } from '../old-name';`, - filename: `/fake/dir/dir/child.js`, + filename: resolve(`./fake/dir/dir/child.js`), output: `import { a } from '../new-name';` }, { code: `import { a } from './fake/dir/old-name';`, - filename: `/parent.js`, + filename: resolve(`./parent.js`), output: `import { a } from './fake/dir/new-name';` } ].map((spec) => ({ ...spec, errors, options })) @@ -111,17 +112,17 @@ describe('when renaming one file', () => { invalid: [ { code: `const { a } = require('./old-name');`, - filename: `/fake/dir/sibling.js`, + filename: resolve(`./fake/dir/sibling.js`), output: `const { a } = require('./new-name');` }, { code: `const { a } = require('../old-name');`, - filename: `/fake/dir/dir/child.js`, + filename: resolve(`./fake/dir/dir/child.js`), output: `const { a } = require('../new-name');` }, { code: `const { a } = require('./fake/dir/old-name');`, - filename: `/parent.js`, + filename: resolve(`./parent.js`), output: `const { a } = require('./fake/dir/new-name');` } ].map((spec) => ({ ...spec, errors, options })) @@ -132,33 +133,38 @@ describe('when renaming one file', () => { [ { - consumer: ['/consumer.js', './fake/old-dir/file', './fake/new-dir/file'], + consumer: ['./consumer.js', './fake/old-dir/file', './fake/new-dir/file'], dependency: ['./dep', '../old-dir/dep'], - filePath: [`/fake/old-dir/file.js`, `/fake/new-dir/file.js`] + filePath: [`./fake/old-dir/file.js`, `./fake/new-dir/file.js`], + target: '../new-dir/file.js' }, { - consumer: ['/fake/dir/lib/consumer.js', '../../file', '../../lib/file'], + consumer: ['./fake/dir/lib/consumer.js', '../../file', '../../lib/file'], dependency: ['./lib/dep', './dep'], - filePath: [`/fake/file.js`, `/fake/lib/file.js`] + filePath: [`./fake/file.js`, `./fake/lib/file.js`], + target: './lib/file.js' }, { - consumer: ['/fake/old-dir/consumer.js', './name', '../new-dir/name'], + consumer: ['./fake/old-dir/consumer.js', './name', '../new-dir/name'], dependency: ['../../dep', '../../dep'], - filePath: [`/fake/old-dir/name.js`, `/fake/new-dir/name.js`] + filePath: [`./fake/old-dir/name.js`, `./fake/new-dir/name.js`], + target: '../new-dir/name.js' }, { - consumer: ['/fake/new-dir/consumer.js', '../old-dir/file', '../../file'], + consumer: ['./fake/new-dir/consumer.js', '../old-dir/file', '../../file'], dependency: ['./dep', './fake/old-dir/dep'], - filePath: [`/fake/old-dir/file.js`, `/file.js`] + filePath: [`./fake/old-dir/file.js`, `./file.js`], + target: '../../file.js' } ].forEach( ({ consumer: [consumerPath, inId, newInId], dependency: [outId, newOutId], - filePath: [oldPath, newPath] + filePath: [oldPath, newPath], + target }) => { describe(`when moving one file from ${oldPath} to ${newPath}`, () => { - const options = [{ files: { [oldPath]: newPath } }]; + const options = [{ files: { [oldPath]: target } }]; const errors = [{ message: `${oldPath} has moved to ${newPath}` }]; const consumerContents = ` import lodash from 'lodash'; @@ -182,7 +188,7 @@ describe('when renaming one file', () => { describe('when file has already been moved', () => { it('is valid', () => { ruleTester.run('move-files', rule, { - valid: [{ code: '', filename: newPath, options }], + valid: [{ code: '', filename: resolve(newPath), options }], invalid: [] }); }); @@ -204,14 +210,14 @@ describe('when renaming one file', () => { { code: fileContents, errors, - filename: oldPath, + filename: resolve(oldPath), options, output: newFileContents }, { code: consumerContents, errors, - filename: consumerPath, + filename: resolve(consumerPath), options, output: newConsumerContents } @@ -225,8 +231,8 @@ describe('when renaming one file', () => { // 2. A file was written in the new location containing the *old* // contents, in reality this would be the new contents with the // updated imports. - expect(existsSync(oldPath)).toEqual(false); - expect(readTextFileSync(newPath)).toEqual(fileContents); + expect(existsSync(resolve(oldPath))).toEqual(false); + expect(readTextFileSync(resolve(newPath))).toEqual(fileContents); done(); }); }); @@ -236,10 +242,10 @@ describe('when renaming one file', () => { ); describe('when moving a glob pattern of multiple files', () => { - const source = '/fake/dir/**/*.js'; + const source = './fake/dir/**/*.js'; describe('when target is another glob pattern', () => { it('throws because it is not clear what to do with them', () => { - ['/fake/other-dir/**/*.js', './**/*.js'].forEach((target: string) => { + ['./fake/other-dir/**/*.js', './**/*.js'].forEach((target: string) => { expect(() => { ruleTester.run('move-files', rule, { valid: [], @@ -279,7 +285,7 @@ describe('when moving a glob pattern of multiple files', () => { const target = './nested'; const changes = [ { - filePath: ['/fake/dir/file-a.js', '/fake/dir/nested/file-a.js'], + filePath: ['./fake/dir/file-a.js', './fake/dir/nested/file-a.js'], contents: [ ` import { b } from './b/file-b'; @@ -294,7 +300,7 @@ describe('when moving a glob pattern of multiple files', () => { ] }, { - filePath: ['/fake/dir/b/file-b.js', '/fake/dir/b/nested/file-b.js'], + filePath: ['./fake/dir/b/file-b.js', './fake/dir/b/nested/file-b.js'], contents: [ ` import { a } from '../file-a'; @@ -309,7 +315,10 @@ describe('when moving a glob pattern of multiple files', () => { ] }, { - filePath: ['/fake/dir/b/c/file-c.js', '/fake/dir/b/c/nested/file-c.js'], + filePath: [ + './fake/dir/b/c/file-c.js', + './fake/dir/b/c/nested/file-c.js' + ], contents: [ ` import { a } from '../../file-a'; @@ -343,7 +352,7 @@ describe('when moving a glob pattern of multiple files', () => { invalid: changes.map(({ contents, filePath }) => ({ code: contents[0], errors: [{ message: `${filePath[0]} has moved to ${filePath[1]}` }], - filename: filePath[0], + filename: resolve(filePath[0]), options: [{ files: { [source]: target } }], output: contents[1] })) @@ -357,8 +366,8 @@ describe('when moving a glob pattern of multiple files', () => { // contents, in reality this would be the new contents with the // updated imports. changes.forEach(({ contents, filePath }) => { - expect(existsSync(filePath[0])).toEqual(false); - expect(readTextFileSync(filePath[1])).toEqual(contents[0]); + expect(existsSync(resolve(filePath[0]))).toEqual(false); + expect(readTextFileSync(resolve(filePath[1]))).toEqual(contents[0]); }); done(); }); @@ -369,7 +378,7 @@ describe('when moving a glob pattern of multiple files', () => { const target = './nested/new-file.js'; const changes = [ { - filePath: ['/fake/dir/file-a.js', '/fake/dir/nested/new-file.js'], + filePath: ['./fake/dir/file-a.js', './fake/dir/nested/new-file.js'], contents: [ ` import { b } from './b/file-b'; @@ -384,7 +393,7 @@ describe('when moving a glob pattern of multiple files', () => { ] }, { - filePath: ['/fake/dir/b/file-b.js', '/fake/dir/b/nested/new-file.js'], + filePath: ['./fake/dir/b/file-b.js', './fake/dir/b/nested/new-file.js'], contents: [ ` import { a } from '../file-a'; @@ -400,8 +409,8 @@ describe('when moving a glob pattern of multiple files', () => { }, { filePath: [ - '/fake/dir/b/c/file-c.js', - '/fake/dir/b/c/nested/new-file.js' + './fake/dir/b/c/file-c.js', + './fake/dir/b/c/nested/new-file.js' ], contents: [ ` @@ -436,7 +445,7 @@ describe('when moving a glob pattern of multiple files', () => { invalid: changes.map(({ contents, filePath }) => ({ code: contents[0], errors: [{ message: `${filePath[0]} has moved to ${filePath[1]}` }], - filename: filePath[0], + filename: resolve(filePath[0]), options: [{ files: { [source]: target } }], output: contents[1] })) @@ -450,8 +459,8 @@ describe('when moving a glob pattern of multiple files', () => { // contents, in reality this would be the new contents with the // updated imports. changes.forEach(({ contents, filePath }) => { - expect(existsSync(filePath[0])).toEqual(false); - expect(readTextFileSync(filePath[1])).toEqual(contents[0]); + expect(existsSync(resolve(filePath[0]))).toEqual(false); + expect(readTextFileSync(resolve(filePath[1]))).toEqual(contents[0]); }); done(); }); diff --git a/src/move-files.ts b/src/move-files.ts index f32f2a0..adcbe47 100644 --- a/src/move-files.ts +++ b/src/move-files.ts @@ -53,7 +53,7 @@ const rule: Rule.RuleModule = { } if (sourceIsGlob && targetIsRelativePath) { - return glob.sync(source).forEach((sourceFile) => { + return glob.sync(source, { absolute: true }).forEach((sourceFile) => { files[sourceFile] = targetIsDirectoryLike ? join(resolve(dirname(sourceFile), target), basename(sourceFile)) : resolve(dirname(sourceFile), target); @@ -65,7 +65,7 @@ const rule: Rule.RuleModule = { } // plain file to plain file - files[source] = target; + files[resolve(source)] = resolve(dirname(source), target); }); const currentFilePath = context.getFilename(); @@ -102,7 +102,7 @@ const rule: Rule.RuleModule = { return; } const newDirPath = dirname(newFilePath); - if (dirPath !== newDirPath) { + if (newDirPath !== dirPath) { const quotes = node.source.raw.charAt(0); const rawModulePath = withFileExtension(resolve(dirPath, moduleId)); const modulePath = files[rawModulePath] || rawModulePath; @@ -122,7 +122,10 @@ const rule: Rule.RuleModule = { // ESLint's types don't reflect that an array of fixes can be returned return (fixes.map((fn) => fn(fixer)) as unknown) as Rule.Fix; }, - message: ERROR_MOVED_FILE(currentFilePath, newFilePath), + message: ERROR_MOVED_FILE( + withLeadingDot(relative(process.cwd(), currentFilePath)), + withLeadingDot(relative(process.cwd(), newFilePath)) + ), node }); } @@ -144,7 +147,10 @@ const rule: Rule.RuleModule = { const withQuotes = `${quotes}${newModuleId}${quotes}`; return context.report({ fix: (fixer) => fixer.replaceText(node.arguments[0], withQuotes), - message: ERROR_MOVED_FILE(modulePath, newModulePath), + message: ERROR_MOVED_FILE( + withLeadingDot(relative(process.cwd(), modulePath)), + withLeadingDot(relative(process.cwd(), newModulePath)) + ), node: node.arguments[0] }); } @@ -163,7 +169,10 @@ const rule: Rule.RuleModule = { const withQuotes = `${quotes}${newModuleId}${quotes}`; return context.report({ fix: (fixer) => fixer.replaceText(node.source, withQuotes), - message: ERROR_MOVED_FILE(modulePath, newModulePath), + message: ERROR_MOVED_FILE( + withLeadingDot(relative(process.cwd(), modulePath)), + withLeadingDot(relative(process.cwd(), newModulePath)) + ), node: node.source }); }