Skip to content

Commit

Permalink
feat(rule): resolve paths relative to project root
Browse files Browse the repository at this point in the history
  • Loading branch information
@JamieMason
JamieMason committed Jul 7, 2019
1 parent bdb44ca commit de2ddcb
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 50 deletions.
97 changes: 53 additions & 44 deletions src/move-files.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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: []
});
});
Expand All @@ -56,7 +57,7 @@ describe('when renaming one file', () => {
{
code: fileContents,
errors,
filename: oldPath,
filename: resolve(oldPath),
options
}
]
Expand All @@ -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 }))
Expand All @@ -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 }))
Expand All @@ -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';
Expand All @@ -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: []
});
});
Expand All @@ -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
}
Expand All @@ -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();
});
});
Expand All @@ -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: [],
Expand Down Expand Up @@ -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';
Expand All @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -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]
}))
Expand All @@ -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();
});
Expand All @@ -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';
Expand All @@ -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';
Expand All @@ -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: [
`
Expand Down Expand Up @@ -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]
}))
Expand All @@ -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();
});
Expand Down
21 changes: 15 additions & 6 deletions src/move-files.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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;
Expand All @@ -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
});
}
Expand All @@ -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]
});
}
Expand All @@ -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
});
}
Expand Down

0 comments on commit de2ddcb

Please sign in to comment.