'move' incorrect files name (angular_devkit/schematics/src/rules/move.ts)

[seneric]

🐞 Bug report

Description

For example when you try use move('/myApp', '/')
at line 33 : tree.rename(path, toPath + '/' + path.substr(fromPath.length));
toPath + '/' + path.substr(fromPath.length) all files contain '///' before there name.

🔬 Minimal Reproduction

tree.create('a/b/file1', 'hello world');
move('a/b', '/') => file name is ///file1

A possible correction:
I replaced line 33:
tree.rename(path, toPath + '/' + path.substr(fromPath.length));

by

if (toPath === '/') {
  tree.rename(path, path.substr(fromPath.length));
} else {
  tree.rename(path, toPath + ((path.substr(fromPath.length))[0] === '/' ? '': '/') + path.substr(fromPath.length));
}

test file:

describe('move', () => {
    it('works on moving the whole structure', done => {
        const tree = new HostTree();
        tree.create('a/b/file1', 'hello world');
        tree.create('a/b/file2', 'hello world');
        tree.create('a/c/file3', 'hello world');

        callRule(move('sub'), observableOf(tree), context)
            .toPromise()
            .then(result => {
                expect(result.exists('sub/a/b/file1')).toBe(true);
                expect(result.exists('sub/a/b/file2')).toBe(true);
                expect(result.exists('sub/a/c/file3')).toBe(true);
            })
            .then(done, done.fail);
    });

    it('works on moving a subdirectory structure', done => {
        const tree = new HostTree();
        tree.create('a/b/file1', 'hello world');
        tree.create('a/b/file2', 'hello world');
        tree.create('a/c/file3', 'hello world');

        callRule(move('a/b', 'sub'), observableOf(tree), context)
            .toPromise()
            .then(result => {
                expect(result.exists('sub/file1')).toBe(true);
                expect(result.exists('sub/file2')).toBe(true);
                expect(result.exists('a/c/file3')).toBe(true);
            })
            .then(done, done.fail);
    });

    it('works on moving a directory into a subdirectory of itself', done => {
        const tree = new HostTree();
        tree.create('a/b/file1', 'hello world');
        tree.create('a/b/file2', 'hello world');
        tree.create('a/c/file3', 'hello world');

        callRule(move('a/b', 'a/b/c'), observableOf(tree), context)
            .toPromise()
            .then(result => {
                expect(result.exists('a/b/c/file1')).toBe(true);
                expect(result.exists('a/b/c/file2')).toBe(true);
                expect(result.exists('a/c/file3')).toBe(true);
            })
            .then(done, done.fail);
    });

    it('works on moving a directory into a parent of itself', done => {
        const tree = new HostTree();
        tree.create('a/b/file1', 'hello world');
        tree.create('a/b/file2', 'hello world');
        tree.create('a/c/file3', 'hello world');

        callRule(move('a/b', 'a'), observableOf(tree), context)
            .toPromise()
            .then(result => {
                expect(result.exists('file1')).toBe(false);
                expect(result.exists('file2')).toBe(false);
                expect(result.exists('a/file1')).toBe(true);
                expect(result.exists('a/file2')).toBe(true);
                expect(result.exists('a/c/file3')).toBe(true);
            })
            .then(done, done.fail);
    });

    it('becomes a noop with identical from and to', done => {
        const tree = new HostTree();
        tree.create('a/b/file1', 'hello world');
        tree.create('a/b/file2', 'hello world');
        tree.create('a/c/file3', 'hello world');

        callRule(move(''), observableOf(tree), context)
            .toPromise()
            .then(result => {
                expect(result.exists('a/b/file1')).toBe(true);
                expect(result.exists('a/b/file2')).toBe(true);
                expect(result.exists('a/c/file3')).toBe(true);
            })
            .then(done, done.fail);
    });

    it('works on moving a directory to root', done => {
        const tree = new HostTree();
        tree.create('a/b/file1', 'hello world');
        tree.create('a/b/file2', 'hello world');
        tree.create('a/c/file3', 'hello world');

        callRule(move('a/b', '/'), observableOf(tree), context)
            .toPromise()
            .then(result => {
                expect(result.exists('/file1')).toBe(true);
                expect(result.exists('/file2')).toBe(true);
                expect(result.exists('/file3')).toBe(false);
                expect(result.exists('a/c/file3')).toBe(true);
            })
            .then(done, done.fail);
    });
});

🌍 Your Environment

all angular-cli versions (even the last "master branch" version)

[alan-agius4]

@seneric, I tried to replicate the issue, and it seems that during the rename the slashes are normalised and deduped properly. Infact trying to run the test you provided passes using the current master.

Maybe you can explain what is the problem that you are experiencing?

That said, in the line you mentioned we should probably use join. See: #13655

[seneric]

when you try use move('/myApp', '/') and debug the code when you spy what is contains in toPath + '/' + path.substr(fromPath.length) you have /// at the begining of the string (and it is not 'clean').
Perhaps using a join is cleaner ...

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}