Skip to content

Commit f616265

Browse files
TwitchBronBronTwitchBronBron
committed
Fix tab issue when printing diagnostics
1 parent 56df4da commit f616265

File tree

3 files changed

+142
-50
lines changed

3 files changed

+142
-50
lines changed

‎src/diagnosticUtils.spec.ts

Lines changed: 111 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,23 @@ import * as diagnosticUtils from './diagnosticUtils';
44
import { Range, DiagnosticSeverity } from 'vscode-languageserver';
55
import { util } from './util';
66
import chalk from 'chalk';
7+
import { createSandbox } from 'sinon';
8+
import undent from 'undent';
9+
import type { BsDiagnostic } from './interfaces';
10+
import { stripConsoleColors } from './testHelpers.spec';
11+
const sinon = createSandbox();
712

813
describe('diagnosticUtils', () => {
914
let options: ReturnType<typeof diagnosticUtils.getPrintDiagnosticOptions>;
1015
beforeEach(() => {
16+
sinon.restore();
1117
options = diagnosticUtils.getPrintDiagnosticOptions({});
1218
});
1319

20+
afterEach(() => {
21+
sinon.restore();
22+
});
23+
1424
describe('printDiagnostic', () => {
1525
it('does not crash when range is undefined', () => {
1626
//print a diagnostic that doesn't have a range...it should not explode
@@ -25,10 +35,73 @@ describe('diagnosticUtils', () => {
2535
//print a diagnostic that doesn't have a range...it should not explode
2636
diagnosticUtils.printDiagnostic(options, DiagnosticSeverity.Error, undefined, [], {
2737
message: 'Bad thing happened',
28-
range: Range.create(0, 0, 2, 2), //important...this needs to be null for the test to pass,
38+
range: Range.create(0, 0, 2, 2),
2939
code: 1234
3040
} as any);
3141
});
42+
43+
function testPrintDiagnostic(diagnostic: BsDiagnostic, code: string, expected: string) {
44+
let logOutput = '';
45+
sinon.stub(console, 'log').callsFake((...args: any[]) => {
46+
if (logOutput.length > 0) {
47+
logOutput += '\n';
48+
}
49+
logOutput += stripConsoleColors(args.join(' '));
50+
});
51+
//print a diagnostic that doesn't have a range...it should not explode
52+
diagnosticUtils.printDiagnostic(options, DiagnosticSeverity.Error, undefined, code.split(/\r?\n/g), diagnostic);
53+
54+
//remove leading and trailing newlines
55+
logOutput = logOutput.replace(/^[\r\n]*/g, '').replace(/[\r\n]*$/g, '');
56+
expected = undent(logOutput).replace(/^[\r\n]*/g, '').replace(/[\r\n]*$/g, '');
57+
58+
expect(logOutput).to.eql(expected);
59+
}
60+
61+
it('handles mixed tabs and spaces', () => {
62+
testPrintDiagnostic(
63+
{
64+
message: 'Bad thing happened',
65+
range: Range.create(0, 5, 0, 18),
66+
code: 1234
67+
} as any,
68+
`\t \t print "hello"`,
69+
`
70+
<unknown file>:1:6 - error BS1234: Bad thing happened
71+
1 print "hello"
72+
_ ~~~~~~~~~~~~~
73+
`);
74+
});
75+
76+
it('handles only tabs', () => {
77+
testPrintDiagnostic(
78+
{
79+
message: 'Bad thing happened',
80+
range: Range.create(0, 5, 0, 18),
81+
code: 1234
82+
} as any,
83+
`\tprint "hello"`,
84+
`
85+
<unknown file>:1:6 - error BS1234: Bad thing happened
86+
1 print "hello"
87+
_ ~~~~~~~~~~~~~
88+
`);
89+
});
90+
91+
it('handles only spaces', () => {
92+
testPrintDiagnostic(
93+
{
94+
message: 'Bad thing happened',
95+
range: Range.create(0, 5, 0, 18),
96+
code: 1234
97+
} as any,
98+
` print "hello"`,
99+
`
100+
<unknown file>:1:6 - error BS1234: Bad thing happened
101+
1 print "hello"
102+
_ ~~~~~~~~~~~~~
103+
`);
104+
});
32105
});
33106

34107
describe('getPrintDiagnosticOptions', () => {
@@ -92,76 +165,75 @@ describe('diagnosticUtils', () => {
92165

93166
describe('getDiagnosticSquiggly', () => {
94167
it('works for normal cases', () => {
95-
expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
96-
range: Range.create(0, 0, 0, 4)
97-
}, 'asdf')).to.equal('~~~~');
168+
expect(
169+
diagnosticUtils.getDiagnosticSquigglyText('asdf', 0, 4)
170+
).to.equal('~~~~');
98171
});
99172

100173
it('highlights whole line if no range', () => {
101-
expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
102-
}, ' asdf ')).to.equal('~~~~~~');
174+
expect(
175+
diagnosticUtils.getDiagnosticSquigglyText(' asdf ', undefined, undefined)
176+
).to.equal('~~~~~~');
103177
});
104178

105179
it('returns empty string when no line is found', () => {
106-
expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
107-
range: Range.create(0, 0, 0, 10)
108-
}, '')).to.equal('');
180+
expect(diagnosticUtils.getDiagnosticSquigglyText('', 0, 10)).to.equal('');
109181

110-
expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
111-
range: Range.create(0, 0, 0, 10)
112-
}, undefined)).to.equal('');
182+
expect(
183+
diagnosticUtils.getDiagnosticSquigglyText(undefined, 0, 10)
184+
).to.equal('');
113185
});
114186

115187
it('supports diagnostic not at start of line', () => {
116-
expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
117-
range: Range.create(0, 2, 0, 6)
118-
}, ' asdf')).to.equal(' ~~~~');
188+
expect(
189+
diagnosticUtils.getDiagnosticSquigglyText(' asdf', 2, 6)
190+
).to.equal(' ~~~~');
119191
});
120192

121193
it('supports diagnostic that does not finish at end of line', () => {
122-
expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
123-
range: Range.create(0, 0, 0, 4)
124-
}, 'asdf ')).to.equal('~~~~ ');
194+
expect(
195+
diagnosticUtils.getDiagnosticSquigglyText('asdf ', 0, 4)
196+
).to.equal('~~~~ ');
125197
});
126198

127199
it('supports diagnostic with space on both sides', () => {
128-
expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
129-
range: Range.create(0, 2, 0, 6)
130-
}, ' asdf ')).to.equal(' ~~~~ ');
200+
expect(
201+
diagnosticUtils.getDiagnosticSquigglyText(' asdf ', 2, 6)
202+
).to.equal(' ~~~~ ');
131203
});
132204

133205
it('handles diagnostic that starts and stops on the same position', () => {
134-
expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
135-
range: Range.create(0, 2, 0, 2)
136-
}, 'abcde')).to.equal('~~~~~');
206+
expect(
207+
diagnosticUtils.getDiagnosticSquigglyText('abcde', 2, 2)
208+
).to.equal('~~~~~');
137209
});
138210

139211
it('handles single-character diagnostic', () => {
140-
expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
141-
range: Range.create(0, 2, 0, 3)
142-
}, 'abcde')).to.equal(' ~ ');
212+
expect(
213+
diagnosticUtils.getDiagnosticSquigglyText('abcde', 2, 3)
214+
).to.equal(' ~ ');
143215
});
144216

145217
it('handles diagnostics that are longer than the line', () => {
146-
expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
147-
range: Range.create(0, 0, 0, 10)
148-
}, 'abcde')).to.equal('~~~~~');
218+
expect(
219+
diagnosticUtils.getDiagnosticSquigglyText('abcde', 0, 10)
220+
).to.equal('~~~~~');
149221

150-
expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
151-
range: Range.create(0, 2, 0, 10)
152-
}, 'abcde')).to.equal(' ~~~');
222+
expect(
223+
diagnosticUtils.getDiagnosticSquigglyText('abcde', 2, 10)
224+
).to.equal(' ~~~');
153225
});
154226

155227
it('handles Number.MAX_VALUE for end character', () => {
156-
expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
157-
range: util.createRange(0, 0, 0, Number.MAX_VALUE)
158-
}, 'abcde')).to.equal('~~~~~');
228+
expect(
229+
diagnosticUtils.getDiagnosticSquigglyText('abcde', 0, Number.MAX_VALUE)
230+
).to.equal('~~~~~');
159231
});
160232

161233
it.skip('handles edge cases', () => {
162-
expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
163-
range: Range.create(5, 16, 5, 18)
164-
}, 'end functionasdf')).to.equal(' ~~~~');
234+
expect(
235+
diagnosticUtils.getDiagnosticSquigglyText('end functionasdf', 16, 18)
236+
).to.equal(' ~~~~');
165237
});
166238
});
167239

‎src/diagnosticUtils.ts

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,20 @@ export function getDiagnosticLine(diagnostic: BsDiagnostic, diagnosticLine: stri
101101
if (diagnostic.range && diagnosticLine) {
102102
const lineNumberText = chalk.bgWhite(' ' + chalk.black((diagnostic.range.start.line + 1).toString()) + ' ') + ' ';
103103
const blankLineNumberText = chalk.bgWhite(' ' + chalk.white('_'.repeat((diagnostic.range.start.line + 1).toString().length)) + ' ') + ' ';
104-
const squigglyText = getDiagnosticSquigglyText(diagnostic, diagnosticLine);
104+
105+
//remove tabs in favor of spaces to make diagnostic printing more consistent
106+
let leadingText = diagnosticLine.slice(0, diagnostic.range.start.character);
107+
let leadingTextNormalized = leadingText.replace(/\t/g, ' ');
108+
let actualText = diagnosticLine.slice(diagnostic.range.start.character, diagnostic.range.end.character);
109+
let actualTextNormalized = actualText.replace(/\t/g, ' ');
110+
let startIndex = leadingTextNormalized.length;
111+
let endIndex = leadingTextNormalized.length + actualTextNormalized.length;
112+
113+
let diagnosticLineNormalized = diagnosticLine.replace(/\t/g, ' ');
114+
115+
const squigglyText = getDiagnosticSquigglyText(diagnosticLineNormalized, startIndex, endIndex);
105116
result +=
106-
lineNumberText + diagnosticLine + '\n' +
117+
lineNumberText + diagnosticLineNormalized + '\n' +
107118
blankLineNumberText + colorFunction(squigglyText);
108119
}
109120
return result;
@@ -112,36 +123,36 @@ export function getDiagnosticLine(diagnostic: BsDiagnostic, diagnosticLine: stri
112123
/**
113124
* Given a diagnostic, compute the range for the squiggly
114125
*/
115-
export function getDiagnosticSquigglyText(diagnostic: BsDiagnostic, line: string) {
126+
export function getDiagnosticSquigglyText(line: string, startCharacter: number, endCharacter: number) {
116127
let squiggle: string;
117128
//fill the entire line
118129
if (
119130
//there is no range
120-
!diagnostic.range ||
131+
typeof startCharacter !== 'number' || typeof endCharacter !== 'number' ||
121132
//there is no line
122133
!line ||
123134
//both positions point to same location
124-
diagnostic.range.start.character === diagnostic.range.end.character ||
135+
startCharacter === endCharacter ||
125136
//the diagnostic starts after the end of the line
126-
diagnostic.range.start.character >= line.length
137+
startCharacter >= line.length
127138
) {
128139
squiggle = ''.padStart(line?.length ?? 0, '~');
129140
} else {
130141

131-
let endIndex = Math.max(diagnostic.range?.end.character, line.length);
142+
let endIndex = Math.max(endCharacter, line.length);
132143
endIndex = endIndex > 0 ? endIndex : 0;
133144
if (line?.length < endIndex) {
134145
endIndex = line.length;
135146
}
136147

137-
let leadingWhitespaceLength = diagnostic.range.start.character;
148+
let leadingWhitespaceLength = startCharacter;
138149
let squiggleLength: number;
139-
if (diagnostic.range.end.character === Number.MAX_VALUE) {
150+
if (endCharacter === Number.MAX_VALUE) {
140151
squiggleLength = line.length - leadingWhitespaceLength;
141152
} else {
142-
squiggleLength = diagnostic.range.end.character - diagnostic.range.start.character;
153+
squiggleLength = endCharacter - startCharacter;
143154
}
144-
let trailingWhitespaceLength = endIndex - diagnostic.range.end.character;
155+
let trailingWhitespaceLength = endIndex - endCharacter;
145156

146157
//opening whitespace
147158
squiggle =

‎src/testHelpers.spec.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,3 +362,12 @@ export function mapToObject<T>(map: Map<any, T>) {
362362
}
363363
return result;
364364
}
365+
366+
export function stripConsoleColors(inputString) {
367+
// Regular expression to match ANSI escape codes for colors
368+
// eslint-disable-next-line no-control-regex
369+
const colorPattern = /\u001b\[(?:\d*;){0,5}\d*m/g;
370+
371+
// Remove all occurrences of ANSI escape codes
372+
return inputString.replace(colorPattern, '');
373+
}

0 commit comments

Comments
 (0)