Skip to content

Drop FileSystem.getRealPath(). #9697

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions src/client/common/platform/fileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,6 @@ export class FileSystem implements IFileSystem {
return this.pathUtils.arePathsSame(path1, path2);
}

public getRealPath(filePath: string): Promise<string> {
return this.pathUtils.getRealPath(filePath);
}

//=================================
// "raw" operations

Expand Down
27 changes: 3 additions & 24 deletions src/client/common/platform/fs-paths.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import * as fs from 'fs-extra';
import * as nodepath from 'path';
import { getOSType, OSType } from '../utils/platform';
// prettier-ignore
import {
IExecutables,
IFileSystemPaths
IFileSystemPaths, IFileSystemPathUtils
} from './types';
// tslint:disable-next-line:no-var-requires no-require-imports
const untildify = require('untildify');
Expand All @@ -21,8 +20,7 @@ interface INodePath {
normalize(filename: string): string;
}

// The file path operations used by the extension.
export class FileSystemPaths {
export class FileSystemPaths implements IFileSystemPaths {
// prettier-ignore
constructor(
private readonly isCaseInsensitive: boolean,
Expand Down Expand Up @@ -74,10 +72,6 @@ export class FileSystemPaths {
}
}

// Where to fine executables.
//
// In particular this class provides all the tools needed to find
// executables, including through an environment variable.
export class Executables {
// prettier-ignore
constructor(
Expand Down Expand Up @@ -108,8 +102,7 @@ interface IRawPaths {
relative(relpath: string, rootpath: string): string;
}

// A collection of high-level utilities related to filesystem paths.
export class FileSystemPathUtils {
export class FileSystemPathUtils implements IFileSystemPathUtils {
// prettier-ignore
constructor(
public readonly home: string,
Expand All @@ -136,26 +129,12 @@ export class FileSystemPathUtils {
);
}

// Return true if the two paths are equivalent on the current
// filesystem and false otherwise. On Windows this is significant.
// On non-Windows the filenames must always be exactly the same.
public arePathsSame(path1: string, path2: string): boolean {
path1 = this.paths.normCase(path1);
path2 = this.paths.normCase(path2);
return path1 === path2;
}

// Return the canonicalized absolute filename.
public async getRealPath(filename: string): Promise<string> {
try {
return await fs.realpath(filename);
} catch {
// We ignore the error.
return filename;
}
}

// Return the clean (displayable) form of the given filename.
public getDisplayName(filename: string, cwd?: string): string {
if (cwd && filename.startsWith(cwd)) {
return `.${this.paths.sep}${this.raw.relative(cwd, filename)}`;
Expand Down
12 changes: 11 additions & 1 deletion src/client/common/platform/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export type TemporaryDirectory = { path: string } & vscode.Disposable;
//===========================
// FS paths

// The low-level file path operations used by the extension.
export interface IFileSystemPaths {
readonly sep: string;
join(...filenames: string[]): string;
Expand All @@ -59,15 +60,25 @@ export interface IFileSystemPaths {
normCase(filename: string): string;
}

// Where to fine executables.
//
// In particular this class provides all the tools needed to find
// executables, including through an environment variable.
export interface IExecutables {
delimiter: string;
envVar: string;
}

// A collection of high-level utilities related to filesystem paths.
export interface IFileSystemPathUtils {
readonly paths: IFileSystemPaths;
readonly executables: IExecutables;
readonly home: string;
// Return true if the two paths are equivalent on the current
// filesystem and false otherwise. On Windows this is significant.
// On non-Windows the filenames must always be exactly the same.
arePathsSame(path1: string, path2: string): boolean;
// Return the clean (displayable) form of the given filename.
getDisplayName(pathValue: string, cwd?: string): string;
}

Expand All @@ -83,7 +94,6 @@ export interface IFileSystem {
// path-related
directorySeparatorChar: string;
arePathsSame(path1: string, path2: string): boolean;
getRealPath(path: string): Promise<string>;

// "raw" operations
stat(filePath: string): Promise<FileStat>;
Expand Down
13 changes: 0 additions & 13 deletions src/test/common/platform/filesystem.functional.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,6 @@ suite('FileSystem', () => {
expect(areSame).to.equal(expected);
});
});

suite('getRealPath', () => {
// tested fully in the FileSystemPathUtils tests.

test('matches wrapped object', async () => {
const filename = fixPath('a/b/c/spam.py');
const expected = await pathUtils.getRealPath(filename);

const resolved = await fileSystem.getRealPath(filename);

expect(resolved).to.equal(expected);
});
});
});

suite('raw', () => {
Expand Down
140 changes: 1 addition & 139 deletions src/test/common/platform/fs-paths.functional.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ import {
FileSystemPaths,
FileSystemPathUtils
} from '../../../client/common/platform/fs-paths';
// prettier-ignore
import {
fixPath, FSFixture, OSX, SUPPORTS_SYMLINKS, WINDOWS as IS_WINDOWS
} from './utils';
import { WINDOWS as IS_WINDOWS } from './utils';

suite('FileSystem - Paths', () => {
let paths: FileSystemPaths;
Expand Down Expand Up @@ -186,10 +183,8 @@ suite('FileSystem - Executables', () => {

suite('FileSystem - Path Utils', () => {
let utils: FileSystemPathUtils;
let fix: FSFixture;
setup(() => {
utils = FileSystemPathUtils.withDefaults();
fix = new FSFixture();
});

suite('arePathsSame', () => {
Expand Down Expand Up @@ -231,139 +226,6 @@ suite('FileSystem - Path Utils', () => {
});
});

suite('getRealPath', () => {
const prevCwd = process.cwd();
let cwd: string;
setup(async function() {
if (OSX) {
// tslint:disable-next-line:no-suspicious-comment
// TODO(GH-8995) These tests are failing on Mac, so
// we are temporarily disabling it.
// tslint:disable-next-line:no-invalid-this
return this.skip();
}
cwd = await fix.createDirectory('x/y/z');
process.chdir(cwd);
});
teardown(() => {
process.chdir(prevCwd);
});

test('basename-only', async () => {
const expected = await fix.createFile('x/y/z/spam.py');

const resolved = await utils.getRealPath('spam.py');

expect(resolved).to.equal(expected);
});

test('absolute', async () => {
const filename = await fix.createFile('spam.py');
const expected = filename;

const resolved = await utils.getRealPath(filename);

expect(resolved).to.equal(expected);
});

test('relative', async () => {
const expected = await fix.createFile('x/y/z/w/spam.py');
const relpath = fixPath('./w/spam.py');

const resolved = await utils.getRealPath(relpath);

expect(resolved).to.equal(expected);
});

test('parent', async () => {
const expected = await fix.resolve('x/y');

const resolved = await utils.getRealPath('..');

expect(resolved).to.equal(expected);
});

test('cousin', async () => {
const expected = await fix.createFile('x/w/spam.py');
const relpath = fixPath('../../w/spam.py');

const resolved = await utils.getRealPath(relpath);

expect(resolved).to.equal(expected);
});

test('does not exist', async () => {
const resolved = await utils.getRealPath('spam.py');

// The original path was returned unchanged.
expect(resolved).to.equal('spam.py'); // instead of <TMP>/x/y/z/spam.py
});

test('directory does not exist', async () => {
const relpath = fixPath('../../w/spam.py');

const resolved = await utils.getRealPath(relpath);

// The original path was returned unchanged.
expect(resolved).to.equal(relpath); // instead of <TMP>/x/w/spam.py
});

test('symlink', async function() {
if (!SUPPORTS_SYMLINKS) {
// tslint:disable-next-line:no-invalid-this
this.skip();
}
const expected = await fix.createFile('spam.py');
await fix.createSymlink('x/y/z/eggs.py', expected);

const resolved = await utils.getRealPath('eggs.py');

expect(resolved).to.equal(expected);
});

test('symlink chain', async function() {
if (!SUPPORTS_SYMLINKS) {
// tslint:disable-next-line:no-invalid-this
this.skip();
}
const expected = await fix.createFile('w/spam.py');
const symlink1 = await fix.createSymlink('x/y/spam.py', expected);
await fix.createSymlink('x/y/z/eggs.py', symlink1);

const resolved = await utils.getRealPath('eggs.py');

expect(resolved).to.equal(expected);
});

test('symlink (target does not exist)', async function() {
if (!SUPPORTS_SYMLINKS) {
// tslint:disable-next-line:no-invalid-this
this.skip();
}
const filename = await fix.resolve('spam.py');
await fix.createSymlink('x/y/z/eggs.py', filename);

const resolved = await utils.getRealPath('eggs.py');

// The original path was returned unchanged.
expect(resolved).to.equal('eggs.py'); // instead of <TMP>/spam.py
});

test('mixed', async function() {
if (!SUPPORTS_SYMLINKS) {
// tslint:disable-next-line:no-invalid-this
this.skip();
}
const expected = await fix.createFile('x/y/w/eggs.py');
await fix.createSymlink('x/w/spam.py', expected);
const relpath = fixPath('../../w/spam.py');

const resolved = await utils.getRealPath(relpath);

expect(resolved).to.equal(expected);
});
});

suite('getDisplayName', () => {
const relname = path.join('spam', 'eggs', 'spam.py');
const cwd = path.resolve(path.sep, 'x', 'y', 'z');
Expand Down
1 change: 0 additions & 1 deletion src/test/configuration/interpreterSelector.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ suite('Interpreters - selector', () => {
workspace = TypeMoq.Mock.ofType<IWorkspaceService>();
fileSystem = TypeMoq.Mock.ofType<IFileSystem>();
fileSystem.setup(x => x.arePathsSame(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())).returns((a: string, b: string) => a === b);
fileSystem.setup(x => x.getRealPath(TypeMoq.It.isAnyString())).returns((a: string) => new Promise(resolve => resolve(a)));
configurationService.setup(x => x.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object);

comparer.setup(c => c.compare(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => 0);
Expand Down