Skip to content

Commit

Permalink
refactor(compiler-cli): avoid free-standing FileSystem functions (#…
Browse files Browse the repository at this point in the history 
…39006)

These free standing functions rely upon the "current" `FileSystem`,
but it is safer to explicitly pass the `FileSystem` into functions or
classes that need it.

PR Close #39006
  • Loading branch information
@petebacondarwin @josephperrott
petebacondarwin authored and josephperrott committed Sep 30, 2020
1 parent 23e2188 commit ac3aa04
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 40 deletions.
12 changes: 7 additions & 5 deletions packages/compiler-cli/src/ngtsc/sourcemaps/src/source_file.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import {removeComments, removeMapFileComments} from 'convert-source-map';
import {decode, encode, SourceMapMappings, SourceMapSegment} from 'sourcemap-codec';

import {AbsoluteFsPath, dirname, relative} from '../../file_system';
import {AbsoluteFsPath, FileSystem} from '../../file_system';

import {RawSourceMap} from './raw_source_map';
import {compareSegments, offsetSegment, SegmentMarker} from './segment_marker';
Expand Down Expand Up @@ -38,7 +38,9 @@ export class SourceFile {
/** Whether this source file's source map was inline or external. */
readonly inline: boolean,
/** Any source files referenced by the raw source map associated with this source file. */
readonly sources: (SourceFile|null)[]) {
readonly sources: (SourceFile|null)[],
private fs: FileSystem,
) {
this.contents = removeSourceMapComments(contents);
this.startOfLinePositions = computeStartOfLinePositions(this.contents);
this.flattenedMappings = this.flattenMappings();
Expand Down Expand Up @@ -75,11 +77,11 @@ export class SourceFile {
mappings[line].push(mappingArray);
}

const sourcePathDir = dirname(this.sourcePath);
const sourcePathDir = this.fs.dirname(this.sourcePath);
const sourceMap: RawSourceMap = {
version: 3,
file: relative(sourcePathDir, this.sourcePath),
sources: sources.map(sf => relative(sourcePathDir, sf.sourcePath)),
file: this.fs.relative(sourcePathDir, this.sourcePath),
sources: sources.map(sf => this.fs.relative(sourcePathDir, sf.sourcePath)),
names,
mappings: encode(mappings),
sourcesContent: sources.map(sf => sf.contents),
Expand Down
6 changes: 3 additions & 3 deletions packages/compiler-cli/src/ngtsc/sourcemaps/src/source_file_loader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/
import {commentRegex, fromComment, mapFileCommentRegex} from 'convert-source-map';

import {absoluteFrom, AbsoluteFsPath, FileSystem} from '../../file_system';
import {AbsoluteFsPath, FileSystem} from '../../file_system';
import {Logger} from '../../logging';

import {RawSourceMap} from './raw_source_map';
Expand Down Expand Up @@ -83,7 +83,7 @@ export class SourceFileLoader {
inline = mapAndPath.mapPath === null;
}

return new SourceFile(sourcePath, contents, map, inline, sources);
return new SourceFile(sourcePath, contents, map, inline, sources, this.fs);
} catch (e) {
this.logger.warn(
`Unable to fully load ${sourcePath} for source-map flattening: ${e.message}`);
Expand Down Expand Up @@ -124,7 +124,7 @@ export class SourceFileLoader {
}
}

const impliedMapPath = absoluteFrom(sourcePath + '.map');
const impliedMapPath = this.fs.resolve(sourcePath + '.map');
if (this.fs.exists(impliedMapPath)) {
return {map: this.readRawSourceMap(impliedMapPath), mapPath: impliedMapPath};
}
Expand Down
69 changes: 37 additions & 32 deletions packages/compiler-cli/src/ngtsc/sourcemaps/test/source_file_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,19 @@
*/
import {encode} from 'sourcemap-codec';

import {absoluteFrom} from '../../file_system';
import {absoluteFrom, FileSystem, getFileSystem} from '../../file_system';
import {runInEachFileSystem} from '../../file_system/testing';
import {RawSourceMap} from '../src/raw_source_map';
import {SegmentMarker} from '../src/segment_marker';
import {computeStartOfLinePositions, ensureOriginalSegmentLinks, extractOriginalSegments, findLastMappingIndexBefore, Mapping, parseMappings, SourceFile} from '../src/source_file';

runInEachFileSystem(() => {
describe('SourceFile and utilities', () => {
let fs: FileSystem;
let _: typeof absoluteFrom;

beforeEach(() => {
fs = getFileSystem();
_ = absoluteFrom;
});

Expand All @@ -40,7 +42,7 @@ runInEachFileSystem(() => {
sources: ['a.js'],
version: 3
};
const originalSource = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, []);
const originalSource = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, [], fs);
const mappings = parseMappings(rawSourceMap, [originalSource], [0, 8]);
expect(mappings).toEqual([
{
Expand Down Expand Up @@ -71,7 +73,8 @@ runInEachFileSystem(() => {

it('should parse the segments in ascending order of original position from the raw source map',
() => {
const originalSource = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, []);
const originalSource =
new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, [], fs);
const rawSourceMap: RawSourceMap = {
mappings: encode([[[0, 0, 0, 0], [2, 0, 0, 3], [4, 0, 0, 2]]]),
names: [],
Expand All @@ -88,8 +91,8 @@ runInEachFileSystem(() => {
});

it('should create separate arrays for each original source file', () => {
const sourceA = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, []);
const sourceB = new SourceFile(_('/foo/src/b.js'), '1234567', null, false, []);
const sourceA = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, [], fs);
const sourceB = new SourceFile(_('/foo/src/b.js'), '1234567', null, false, [], fs);
const rawSourceMap: RawSourceMap = {
mappings:
encode([[[0, 0, 0, 0], [2, 1, 0, 3], [4, 0, 0, 2], [5, 1, 0, 5], [6, 1, 0, 2]]]),
Expand Down Expand Up @@ -313,8 +316,8 @@ runInEachFileSystem(() => {
describe('ensureOriginalSegmentLinks', () => {
it('should add `next` properties to each segment that point to the next segment in the same source file',
() => {
const sourceA = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, []);
const sourceB = new SourceFile(_('/foo/src/b.js'), '1234567', null, false, []);
const sourceA = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, [], fs);
const sourceB = new SourceFile(_('/foo/src/b.js'), '1234567', null, false, [], fs);
const rawSourceMap: RawSourceMap = {
mappings:
encode([[[0, 0, 0, 0], [2, 1, 0, 3], [4, 0, 0, 2], [5, 1, 0, 5], [6, 1, 0, 2]]]),
Expand All @@ -336,14 +339,14 @@ runInEachFileSystem(() => {
describe('flattenedMappings', () => {
it('should be an empty array for source files with no source map', () => {
const sourceFile =
new SourceFile(_('/foo/src/index.js'), 'index contents', null, false, []);
new SourceFile(_('/foo/src/index.js'), 'index contents', null, false, [], fs);
expect(sourceFile.flattenedMappings).toEqual([]);
});

it('should be empty array for source files with no source map mappings', () => {
const rawSourceMap: RawSourceMap = {mappings: '', names: [], sources: [], version: 3};
const sourceFile =
new SourceFile(_('/foo/src/index.js'), 'index contents', rawSourceMap, false, []);
new SourceFile(_('/foo/src/index.js'), 'index contents', rawSourceMap, false, [], fs);
expect(sourceFile.flattenedMappings).toEqual([]);
});

Expand All @@ -355,25 +358,26 @@ runInEachFileSystem(() => {
sources: ['a.js'],
version: 3
};
const originalSource = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, []);
const originalSource =
new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, [], fs);
const sourceFile = new SourceFile(
_('/foo/src/index.js'), 'abc123defg', rawSourceMap, false, [originalSource]);
_('/foo/src/index.js'), 'abc123defg', rawSourceMap, false, [originalSource], fs);
expect(removeOriginalSegmentLinks(sourceFile.flattenedMappings))
.toEqual(parseMappings(rawSourceMap, [originalSource], [0, 11]));
});

it('should merge mappings from flattened original source files', () => {
const cSource = new SourceFile(_('/foo/src/c.js'), 'bcd123', null, false, []);
const dSource = new SourceFile(_('/foo/src/d.js'), 'aef', null, false, []);
const cSource = new SourceFile(_('/foo/src/c.js'), 'bcd123', null, false, [], fs);
const dSource = new SourceFile(_('/foo/src/d.js'), 'aef', null, false, [], fs);

const bSourceMap: RawSourceMap = {
mappings: encode([[[0, 1, 0, 0], [1, 0, 0, 0], [4, 1, 0, 1]]]),
names: [],
sources: ['c.js', 'd.js'],
version: 3
};
const bSource =
new SourceFile(_('/foo/src/b.js'), 'abcdef', bSourceMap, false, [cSource, dSource]);
const bSource = new SourceFile(
_('/foo/src/b.js'), 'abcdef', bSourceMap, false, [cSource, dSource], fs);

const aSourceMap: RawSourceMap = {
mappings: encode([[[0, 0, 0, 0], [2, 0, 0, 3], [4, 0, 0, 2], [5, 0, 0, 5]]]),
Expand All @@ -382,7 +386,7 @@ runInEachFileSystem(() => {
version: 3
};
const aSource =
new SourceFile(_('/foo/src/a.js'), 'abdecf', aSourceMap, false, [bSource]);
new SourceFile(_('/foo/src/a.js'), 'abdecf', aSourceMap, false, [bSource], fs);

expect(removeOriginalSegmentLinks(aSource.flattenedMappings)).toEqual([
{
Expand Down Expand Up @@ -431,15 +435,16 @@ runInEachFileSystem(() => {
sources: ['c.js'],
version: 3
};
const bSource = new SourceFile(_('/foo/src/b.js'), 'abcdef', bSourceMap, false, [null]);
const bSource =
new SourceFile(_('/foo/src/b.js'), 'abcdef', bSourceMap, false, [null], fs);
const aSourceMap: RawSourceMap = {
mappings: encode([[[0, 0, 0, 0], [2, 0, 0, 3], [4, 0, 0, 2], [5, 0, 0, 5]]]),
names: [],
sources: ['b.js'],
version: 3
};
const aSource =
new SourceFile(_('/foo/src/a.js'), 'abdecf', aSourceMap, false, [bSource]);
new SourceFile(_('/foo/src/a.js'), 'abdecf', aSourceMap, false, [bSource], fs);

// These flattened mappings are just the mappings from a to b.
// (The mappings to c are dropped since there is no source file to map to.)
Expand All @@ -462,23 +467,23 @@ runInEachFileSystem(() => {

describe('renderFlattenedSourceMap()', () => {
it('should convert the flattenedMappings into a raw source-map object', () => {
const cSource = new SourceFile(_('/foo/src/c.js'), 'bcd123e', null, false, []);
const cSource = new SourceFile(_('/foo/src/c.js'), 'bcd123e', null, false, [], fs);
const bToCSourceMap: RawSourceMap = {
mappings: encode([[[1, 0, 0, 0], [4, 0, 0, 3], [4, 0, 0, 6], [5, 0, 0, 7]]]),
names: [],
sources: ['c.js'],
version: 3
};
const bSource =
new SourceFile(_('/foo/src/b.js'), 'abcdef', bToCSourceMap, false, [cSource]);
new SourceFile(_('/foo/src/b.js'), 'abcdef', bToCSourceMap, false, [cSource], fs);
const aToBSourceMap: RawSourceMap = {
mappings: encode([[[0, 0, 0, 0], [2, 0, 0, 3], [4, 0, 0, 2], [5, 0, 0, 5]]]),
names: [],
sources: ['b.js'],
version: 3
};
const aSource =
new SourceFile(_('/foo/src/a.js'), 'abdecf', aToBSourceMap, false, [bSource]);
new SourceFile(_('/foo/src/a.js'), 'abdecf', aToBSourceMap, false, [bSource], fs);

const aTocSourceMap = aSource.renderFlattenedSourceMap();
expect(aTocSourceMap.version).toEqual(3);
Expand All @@ -493,7 +498,7 @@ runInEachFileSystem(() => {
});

it('should handle mappings that map from lines outside of the actual content lines', () => {
const bSource = new SourceFile(_('/foo/src/b.js'), 'abcdef', null, false, []);
const bSource = new SourceFile(_('/foo/src/b.js'), 'abcdef', null, false, [], fs);
const aToBSourceMap: RawSourceMap = {
mappings: encode([
[[0, 0, 0, 0], [2, 0, 0, 3], [4, 0, 0, 2], [5, 0, 0, 5]],
Expand All @@ -506,7 +511,7 @@ runInEachFileSystem(() => {
version: 3
};
const aSource =
new SourceFile(_('/foo/src/a.js'), 'abdecf', aToBSourceMap, false, [bSource]);
new SourceFile(_('/foo/src/a.js'), 'abdecf', aToBSourceMap, false, [bSource], fs);

const aTocSourceMap = aSource.renderFlattenedSourceMap();
expect(aTocSourceMap.version).toEqual(3);
Expand All @@ -522,13 +527,13 @@ runInEachFileSystem(() => {
describe('getOriginalLocation()', () => {
it('should return null for source files with no flattened mappings', () => {
const sourceFile =
new SourceFile(_('/foo/src/index.js'), 'index contents', null, false, []);
new SourceFile(_('/foo/src/index.js'), 'index contents', null, false, [], fs);
expect(sourceFile.getOriginalLocation(1, 1)).toEqual(null);
});

it('should return offset locations in multiple flattened original source files', () => {
const cSource = new SourceFile(_('/foo/src/c.js'), 'bcd123', null, false, []);
const dSource = new SourceFile(_('/foo/src/d.js'), 'aef', null, false, []);
const cSource = new SourceFile(_('/foo/src/c.js'), 'bcd123', null, false, [], fs);
const dSource = new SourceFile(_('/foo/src/d.js'), 'aef', null, false, [], fs);

const bSourceMap: RawSourceMap = {
mappings: encode([
Expand All @@ -542,8 +547,8 @@ runInEachFileSystem(() => {
sources: ['c.js', 'd.js'],
version: 3
};
const bSource =
new SourceFile(_('/foo/src/b.js'), 'abcdef', bSourceMap, false, [cSource, dSource]);
const bSource = new SourceFile(
_('/foo/src/b.js'), 'abcdef', bSourceMap, false, [cSource, dSource], fs);

const aSourceMap: RawSourceMap = {
mappings: encode([
Expand All @@ -560,7 +565,7 @@ runInEachFileSystem(() => {
version: 3
};
const aSource =
new SourceFile(_('/foo/src/a.js'), 'abde\n cf', aSourceMap, false, [bSource]);
new SourceFile(_('/foo/src/a.js'), 'abde\n cf', aSourceMap, false, [bSource], fs);

// Line 0
expect(aSource.getOriginalLocation(0, 0)) // a
Expand Down Expand Up @@ -592,8 +597,8 @@ runInEachFileSystem(() => {
});

it('should return offset locations across multiple lines', () => {
const originalSource =
new SourceFile(_('/foo/src/original.js'), 'abcdef\nghijk\nlmnop', null, false, []);
const originalSource = new SourceFile(
_('/foo/src/original.js'), 'abcdef\nghijk\nlmnop', null, false, [], fs);
const generatedSourceMap: RawSourceMap = {
mappings: encode([
[
Expand All @@ -614,7 +619,7 @@ runInEachFileSystem(() => {
};
const generatedSource = new SourceFile(
_('/foo/src/generated.js'), 'ABC\nGHIJDEFK\nLMNOP', generatedSourceMap, false,
[originalSource]);
[originalSource], fs);

// Line 0
expect(generatedSource.getOriginalLocation(0, 0)) // A
Expand Down

0 comments on commit ac3aa04

Please sign in to comment.