Skip to content

Commit bf3cfd9

Browse files
aeschliaeschli
committed
Format Document Completely breaks code!!!. Fixes microsoft#19813 
1 parent 63a6969 commit bf3cfd9

6 files changed

Lines changed: 122 additions & 13 deletions

File tree

‎extensions/html/server/src/modes/javascriptMode.ts‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,19 @@ export function getJavascriptMode(documentRegions: LanguageModelCache<HTMLDocume
2323

2424
let compilerOptions: ts.CompilerOptions = { allowNonTsExtensions: true, allowJs: true, target: ts.ScriptTarget.Latest, moduleResolution: ts.ModuleResolutionKind.Classic };
2525
let currentTextDocument: TextDocument;
26+
let scriptFileVersion: number = 0;
27+
function updateCurrentTextDocument(doc: TextDocument) {
28+
if (!currentTextDocument || doc.uri !== currentTextDocument.uri || doc.version !== currentTextDocument.version) {
29+
currentTextDocument = jsDocuments.get(doc);
30+
scriptFileVersion++;
31+
}
32+
}
2633
let host = {
2734
getCompilationSettings: () => compilerOptions,
2835
getScriptFileNames: () => [FILE_NAME, JQUERY_D_TS],
2936
getScriptVersion: (fileName: string) => {
3037
if (fileName === FILE_NAME) {
31-
return String(currentTextDocument.version);
38+
return String(scriptFileVersion);
3239
}
3340
return '1'; // default lib an jquery.d.ts are static
3441
},
@@ -62,7 +69,7 @@ export function getJavascriptMode(documentRegions: LanguageModelCache<HTMLDocume
6269
settings = options && options.javascript;
6370
},
6471
doValidation(document: TextDocument): Diagnostic[] {
65-
currentTextDocument = jsDocuments.get(document);
72+
updateCurrentTextDocument(document);
6673
const diagnostics = jsLanguageService.getSyntacticDiagnostics(FILE_NAME);
6774
return diagnostics.map((diag): Diagnostic => {
6875
return {
@@ -73,7 +80,7 @@ export function getJavascriptMode(documentRegions: LanguageModelCache<HTMLDocume
7380
});
7481
},
7582
doComplete(document: TextDocument, position: Position): CompletionList {
76-
currentTextDocument = jsDocuments.get(document);
83+
updateCurrentTextDocument(document);
7784
let offset = currentTextDocument.offsetAt(position);
7885
let completions = jsLanguageService.getCompletionsAtPosition(FILE_NAME, offset);
7986
if (!completions) {
@@ -100,7 +107,7 @@ export function getJavascriptMode(documentRegions: LanguageModelCache<HTMLDocume
100107
};
101108
},
102109
doResolve(document: TextDocument, item: CompletionItem): CompletionItem {
103-
currentTextDocument = jsDocuments.get(document);
110+
updateCurrentTextDocument(document);
104111
let details = jsLanguageService.getCompletionEntryDetails(FILE_NAME, item.data.offset, item.label);
105112
if (details) {
106113
item.detail = ts.displayPartsToString(details.displayParts);
@@ -110,7 +117,7 @@ export function getJavascriptMode(documentRegions: LanguageModelCache<HTMLDocume
110117
return item;
111118
},
112119
doHover(document: TextDocument, position: Position): Hover {
113-
currentTextDocument = jsDocuments.get(document);
120+
updateCurrentTextDocument(document);
114121
let info = jsLanguageService.getQuickInfoAtPosition(FILE_NAME, currentTextDocument.offsetAt(position));
115122
if (info) {
116123
let contents = ts.displayPartsToString(info.displayParts);
@@ -122,7 +129,7 @@ export function getJavascriptMode(documentRegions: LanguageModelCache<HTMLDocume
122129
return null;
123130
},
124131
doSignatureHelp(document: TextDocument, position: Position): SignatureHelp {
125-
currentTextDocument = jsDocuments.get(document);
132+
updateCurrentTextDocument(document);
126133
let signHelp = jsLanguageService.getSignatureHelpItems(FILE_NAME, currentTextDocument.offsetAt(position));
127134
if (signHelp) {
128135
let ret: SignatureHelp = {
@@ -159,7 +166,7 @@ export function getJavascriptMode(documentRegions: LanguageModelCache<HTMLDocume
159166
return null;
160167
},
161168
findDocumentHighlight(document: TextDocument, position: Position): DocumentHighlight[] {
162-
currentTextDocument = jsDocuments.get(document);
169+
updateCurrentTextDocument(document);
163170
let occurrences = jsLanguageService.getOccurrencesAtPosition(FILE_NAME, currentTextDocument.offsetAt(position));
164171
if (occurrences) {
165172
return occurrences.map(entry => {
@@ -172,7 +179,7 @@ export function getJavascriptMode(documentRegions: LanguageModelCache<HTMLDocume
172179
return null;
173180
},
174181
findDocumentSymbols(document: TextDocument): SymbolInformation[] {
175-
currentTextDocument = jsDocuments.get(document);
182+
updateCurrentTextDocument(document);
176183
let items = jsLanguageService.getNavigationBarItems(FILE_NAME);
177184
if (items) {
178185
let result: SymbolInformation[] = [];
@@ -208,7 +215,7 @@ export function getJavascriptMode(documentRegions: LanguageModelCache<HTMLDocume
208215
return null;
209216
},
210217
findDefinition(document: TextDocument, position: Position): Definition {
211-
currentTextDocument = jsDocuments.get(document);
218+
updateCurrentTextDocument(document);
212219
let definition = jsLanguageService.getDefinitionAtPosition(FILE_NAME, currentTextDocument.offsetAt(position));
213220
if (definition) {
214221
return definition.filter(d => d.fileName === FILE_NAME).map(d => {
@@ -221,7 +228,7 @@ export function getJavascriptMode(documentRegions: LanguageModelCache<HTMLDocume
221228
return null;
222229
},
223230
findReferences(document: TextDocument, position: Position): Location[] {
224-
currentTextDocument = jsDocuments.get(document);
231+
updateCurrentTextDocument(document);
225232
let references = jsLanguageService.getReferencesAtPosition(FILE_NAME, currentTextDocument.offsetAt(position));
226233
if (references) {
227234
return references.filter(d => d.fileName === FILE_NAME).map(d => {
@@ -234,7 +241,7 @@ export function getJavascriptMode(documentRegions: LanguageModelCache<HTMLDocume
234241
return null;
235242
},
236243
format(document: TextDocument, range: Range, formatParams: FormattingOptions): TextEdit[] {
237-
currentTextDocument = jsDocuments.get(document);
244+
updateCurrentTextDocument(document);
238245
let initialIndentLevel = computeInitialIndent(document, range, formatParams);
239246
let formatSettings = convertOptions(formatParams, settings && settings.format, initialIndentLevel + 1);
240247
let start = currentTextDocument.offsetAt(range.start);

‎extensions/html/server/src/test/fixtures/expected/19813-4spaces.html‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<!DOCTYPE html>
2+
<html>
3+
4+
<head>
5+
<meta charset="utf-8">
6+
<script>
7+
Polymer({
8+
is: "chat-messages",
9+
properties: {
10+
user: {},
11+
friend: {
12+
observer: "_friendChanged"
13+
}
14+
},
15+
});
16+
17+
</script>
18+
</head>
19+
20+
<body> </body>
21+
22+
</html>

‎extensions/html/server/src/test/fixtures/expected/19813-tab.html‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<!DOCTYPE html>
2+
<html>
3+
4+
<head>
5+
<meta charset="utf-8">
6+
<script>
7+
Polymer({
8+
is: "chat-messages",
9+
properties: {
10+
user: {},
11+
friend: {
12+
observer: "_friendChanged"
13+
}
14+
},
15+
});
16+
17+
</script>
18+
</head>
19+
20+
<body> </body>
21+
22+
</html>

‎extensions/html/server/src/test/fixtures/expected/19813.html‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<!DOCTYPE html>
2+
<html>
3+
4+
<head>
5+
<meta charset="utf-8">
6+
<script>
7+
Polymer({
8+
is: "chat-messages",
9+
properties: {
10+
user: {},
11+
friend: {
12+
observer: "_friendChanged"
13+
}
14+
},
15+
});
16+
17+
</script>
18+
</head>
19+
20+
<body> </body>
21+
22+
</html>

‎extensions/html/server/src/test/fixtures/inputs/19813.html‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8">
5+
<script>
6+
Polymer({
7+
is: "chat-messages",
8+
properties: {
9+
user: {},
10+
friend: {
11+
observer: "_friendChanged"
12+
}
13+
},
14+
});
15+
</script>
16+
</head>
17+
<body>
18+
</body>
19+
</html>

‎extensions/html/server/src/test/formatting.test.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
*--------------------------------------------------------------------------------------------*/
55
'use strict';
66

7+
import * as path from 'path'
8+
import * as fs from 'fs'
9+
710
import * as assert from 'assert';
811
import { getLanguageModes } from '../modes/languageModes';
912
import { TextDocument, Range, TextEdit, FormattingOptions } from 'vscode-languageserver-types';
@@ -12,7 +15,7 @@ import { format } from '../modes/formatting';
1215

1316
suite('HTML Embedded Formatting', () => {
1417

15-
function assertFormat(value: string, expected: string, options?: any): void {
18+
function assertFormat(value: string, expected: string, options?: any, formatOptions?: FormattingOptions): void {
1619
var languageModes = getLanguageModes({ css: true, javascript: true });
1720
if (options) {
1821
languageModes.getAllModes().forEach(m => m.configure(options));
@@ -31,14 +34,22 @@ suite('HTML Embedded Formatting', () => {
3134
}
3235
let document = TextDocument.create('test://test/test.html', 'html', 0, value);
3336
let range = Range.create(document.positionAt(rangeStartOffset), document.positionAt(rangeEndOffset));
34-
let formatOptions = FormattingOptions.create(2, true);
37+
if (!formatOptions) {
38+
formatOptions = FormattingOptions.create(2, true);
39+
}
3540

3641
let result = format(languageModes, document, range, formatOptions, { css: true, javascript: true });
3742

3843
let actual = applyEdits(document, result);
3944
assert.equal(actual, expected);
4045
}
4146

47+
function assertFormatWithFixture(fixtureName: string, expectedPath: string, options?: any, formatOptions?: FormattingOptions): void {
48+
let input = fs.readFileSync(path.join(__dirname, 'fixtures', 'inputs', fixtureName)).toString();
49+
let expected = fs.readFileSync(path.join(__dirname, 'fixtures', 'expected', expectedPath)).toString();
50+
assertFormat(input, expected, options, formatOptions);
51+
}
52+
4253
test('HTML only', function (): any {
4354
assertFormat('<html><body><p>Hello</p></body></html>', '<html>\n\n<body>\n <p>Hello</p>\n</body>\n\n</html>');
4455
assertFormat('|<html><body><p>Hello</p></body></html>|', '<html>\n\n<body>\n <p>Hello</p>\n</body>\n\n</html>');
@@ -56,6 +67,12 @@ suite('HTML Embedded Formatting', () => {
5667
assertFormat('<html><head>\n <script>\n|var x=6;|\n</script></head></html>', '<html><head>\n <script>\n var x = 6;\n</script></head></html>');
5768
});
5869

70+
test('HTLM & Scripts - Fixtures', function() {
71+
assertFormatWithFixture('19813.html', '19813.html');
72+
assertFormatWithFixture('19813.html', '19813-4spaces.html', void 0, FormattingOptions.create(4, true));
73+
assertFormatWithFixture('19813.html', '19813-tab.html', void 0, FormattingOptions.create(1, false));
74+
})
75+
5976
test('Script end tag', function (): any {
6077
assertFormat('<html>\n<head>\n <script>\nvar x = 0;\n</script></head></html>', '<html>\n\n<head>\n <script>\n var x = 0;\n\n </script>\n</head>\n\n</html>');
6178
});

0 commit comments

Comments
 (0)