Skip to content

Commit

Permalink
fix positioning of comment blocks
Browse files Browse the repository at this point in the history 
now no more double backtick or leading single line # is necessary - everything should be positioned where it belongs in output JS. This doesn't apply to actual CS compiler output of course, but for internal JSDoc intellisense it works well.
Solved internally by inserting the # lines where necessary. This required new logic for remembering inserted lines and then adjusting sourcemaps and fake_line afterwards.
fixes #1
  • Loading branch information
@phil294
phil294 committed Aug 6, 2023
1 parent 54a60a9 commit cad5151
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 35 deletions.
61 changes: 51 additions & 10 deletions server/src/services/transpileService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ const transpilation_cache: Map<string,ITranspilationResult> = new VolatileMap(18
/** The resulting coffee must still always be valid and parsable by the compiler,
and should not shift characters around much (otherwise source maps would need changes too) */
function preprocess_coffee(coffee_doc: TextDocument) {
const tmp = coffee_doc.getText()
const tmp = (coffee_doc.getText() as string)
// Enable autocomplete at `@|`. Replace with magic snippet that allows for both @|
// and standalone @ sign. Cursor needs to be adjusted properly in doComplete().
// .____CoffeeSenseAtSign is replaced with (this.valueOf(),this) in postprocess_js.
Expand Down Expand Up @@ -115,7 +115,32 @@ function preprocess_coffee(coffee_doc: TextDocument) {
logger.logDebug(`replace trailing comma inside { } with ↯ ${coffee_doc.uri}`)
return `,↯${ws}}`
})

const tmp_lines = tmp.split('\n')

const starts_with_block_comment_lines = tmp_lines.map((line) =>
line.match(/^\s*###([^#]|$)/mg)
).map((match, line_i) =>
match ? line_i : -1
).filter(line_i =>
line_i > -1
).map((line_i, i) =>
// Eg. if block comment lines were detected and prefixed with a single # line
// at i=2 and i=4, the array will contain [2,5] so we can iterate and modify
// arrays from the beginning here and in postprocess_js.
line_i + i)

for(const line_i of starts_with_block_comment_lines) {
// Arrange for block comments to be placed directly before their below line in JS (#1)
// Inserts extra lines that need to be tracked so the source maps can be adjusted. That's
// also why this needs to happen before object_tweak_coffee_lines.
// Couldn't find a solution that does not insert extra lines:
// - prefix all ### with another "# " -> long block comment sections become code
// - prefix with backticks: ### -> ``### -> fails inside objects or multiline assignments
logger.logDebug(`replace: prefix ### with single # line ${coffee_doc.uri}`)
tmp_lines.splice(line_i, 0, '#')
}

const object_tweak_coffee_lines: number[] = []
tmp_lines.forEach((line, line_i) => {
// Enable autocomplete on empty lines inside object properties.
Expand All @@ -137,12 +162,14 @@ function preprocess_coffee(coffee_doc: TextDocument) {
if(next_textual_line_indentation > empty_line_indentation.length)
return
}
logger.logDebug(`replace append empty line with 𒐛:𒐛 ${coffee_doc.uri}`)
tmp_lines[line_i] = empty_line_indentation + '𒐛:𒐛'
object_tweak_coffee_lines.push(line_i)
}
})
const coffee = tmp_lines.join('\n')
return { coffee, object_tweak_coffee_lines }
const inserted_coffee_lines = starts_with_block_comment_lines
return { coffee, inserted_coffee_lines, object_tweak_coffee_lines }
}

/** further transforms that *can break* cs compilation, to be used if compilation could not succeed without it anyway */
Expand Down Expand Up @@ -263,7 +290,7 @@ const try_translate_coffee = (coffee_doc: TextDocument): ITranspilationResult =>
} else {
fake_line_mechanism = 'coffee_in_js'
normal_compilation_diagnostics = result.diagnostics

coffee_error_line_no = result.diagnostics![0]!.range.start.line
coffee_error_offset = coffee_doc.offsetAt(Position.create(coffee_error_line_no, 0))
const coffee_error_next_newline_position = coffee.slice(coffee_error_offset).indexOf('\n')
Expand Down Expand Up @@ -410,7 +437,7 @@ const try_translate_coffee = (coffee_doc: TextDocument): ITranspilationResult =>
* Applies some transformations to the JS in result and updates source_map accordingly.
* These transforms do not depend on any previous information.
*/
function postprocess_js(result: ITranspilationResult, object_tweak_coffee_lines: number[]) {
function postprocess_js(result: ITranspilationResult, object_tweak_coffee_lines: number[], inserted_coffee_lines: number[]) {
if(!result.js || !result.source_map)
return

Expand Down Expand Up @@ -442,9 +469,23 @@ function postprocess_js(result: ITranspilationResult, object_tweak_coffee_lines:
source_map_entry.column = 0
}
}

let skip_lines_count = inserted_coffee_lines.findIndex(inserted_line_i =>
source_map_entry.sourceLine < inserted_line_i)
if(skip_lines_count < 0)
skip_lines_count = inserted_coffee_lines.length
source_map_entry.sourceLine -= skip_lines_count
})
})

if(result.fake_line) {
let fake_line_skip_lines_count = inserted_coffee_lines.findIndex(inserted_line_i =>
result.fake_line! < inserted_line_i)
if(fake_line_skip_lines_count < 0)
fake_line_skip_lines_count = inserted_coffee_lines.length
result.fake_line -= fake_line_skip_lines_count
}

// console.time('var-decl-fix')
//////////////////////////////////////
///////// Modify variable declarations to solve various TS compiler errors:
Expand All @@ -461,7 +502,7 @@ function postprocess_js(result: ITranspilationResult, object_tweak_coffee_lines:
// var a;
// a = 1;
// a = 'one';
/////// and now `a` is of type `number | string` (https://github.com/microsoft/TypeScript/issues/45369).
/////// and now `a` is of type `number | string` (https://github.com/microsoft/TypeScript/issues/45369).
// Below is a hacky workaround that should fix these issues in most cases. It moves the
// declaration part (`var`) down to the variable's first implementation position.
// This works only with easy implementations and single-variable array destructuring:
Expand Down Expand Up @@ -592,7 +633,7 @@ const transpile_service: ITranspileService = {
return cached
}

const { coffee: preprocessed_coffee, object_tweak_coffee_lines } = preprocess_coffee(orig_coffee_doc)
const { coffee: preprocessed_coffee, object_tweak_coffee_lines, inserted_coffee_lines } = preprocess_coffee(orig_coffee_doc)
// As coffee was modified, offsets and positions are changed and for these purposes,
// we need to construct a new doc
let mod_coffee_doc = TextDocument.create(orig_coffee_doc.uri, 'coffeescript', 1, preprocessed_coffee)
Expand All @@ -605,7 +646,7 @@ const transpile_service: ITranspileService = {
}

if(result.js && result.source_map) {
postprocess_js(result, object_tweak_coffee_lines)
postprocess_js(result, object_tweak_coffee_lines, inserted_coffee_lines)
} else {
// Nothing worked at all. As a last resort, just pass the coffee to tsserver,
// with minimal transforms:
Expand Down Expand Up @@ -657,7 +698,7 @@ const transpile_service: ITranspileService = {
line_i++
}
}

if(mapped)
coffee_pos = Position.create(mapped.sourceLine, mapped.sourceColumn)
else
Expand Down Expand Up @@ -714,7 +755,7 @@ const transpile_service: ITranspileService = {
}
}
}

// TODO: revise this function, maybe this should be always all line matches by default instead
const get_fitting_js_matches = () => {
const js_matches_by_line = result.source_map!
Expand Down Expand Up @@ -805,7 +846,7 @@ const transpile_service: ITranspileService = {
return abcdefg(js_matches)
}
const js_match = choose_js_match()

let line = js_match?.line
let column = js_match?.column
if(js_match && line != null && result.fake_line == coffee_position.line && result.fake_line_mechanism === 'coffee_in_js') {
Expand Down
5 changes: 3 additions & 2 deletions test/diagnosticHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ import { sleep } from './util';
import { showFile } from './editorHelper';
import { performance } from 'perf_hooks';

export async function testDiagnostics(docUri: vscode.Uri, expectedDiagnostics: vscode.Diagnostic[]) {
export async function testDiagnostics(docUri: vscode.Uri, expectedDiagnostics: vscode.Diagnostic[], allow_unspecified = false) {
await showFile(docUri);

const result = await getDiagnosticsAndTimeout(docUri);

assert.equal(expectedDiagnostics.length, result.length);
if(!allow_unspecified)
assert.equal(expectedDiagnostics.length, result.length);

expectedDiagnostics.forEach(ed => {
assert.ok(
Expand Down
43 changes: 22 additions & 21 deletions test/lsp/features/diagnostics/basic.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { testDiagnostics } from '../../../diagnosticHelper'
import { sameLineRange } from '../../../util'
import { getDocUri } from '../../path'

const string_to_number_error = "Type 'string' is not assignable to type 'number'."

describe('Should find diagnostics', () => {

it('shows diagnostic errors for coffeescript compilation errors', async () => {
Expand All @@ -23,7 +25,7 @@ describe('Should find diagnostics', () => {
range: sameLineRange(2, 0, 14),
severity: vscode.DiagnosticSeverity.Error,
// This also tests basic "pushes down variable declaration to assignment" logic, as the type is fixed number, not number|string as it would be with classical cs compiler (declaration at file head)
message: "Type 'string' is not assignable to type 'number'."
message: string_to_number_error
},
{
range: sameLineRange(6, 14, 17),
Expand Down Expand Up @@ -54,32 +56,31 @@ describe('Should find diagnostics', () => {
{
range: sameLineRange(7, 0, 20),
severity: vscode.DiagnosticSeverity.Error,
message: "Type 'string' is not assignable to type 'number'."
message: string_to_number_error
}
])
})

// TODO: go back to new CS branch, https://github.com/jashkenas/coffeescript/issues/5366#issuecomment-1021366654
// TODO: go back to new CS branch, https://github.com/jashkenas/coffeescript/issues/5366#issuecomment-1021366654 < outdated?
it('pushes down variable declaration to assignment even with comment block before it', async () => {
const docUri = getDocUri('diagnostics/declaration-with-commentblock.coffee')
await testDiagnostics(docUri, [
{
range: sameLineRange(3, 0, 15),
severity: vscode.DiagnosticSeverity.Error,
message: "Type 'string' is not assignable to type 'number'."
}
])
})

// TODO: issue #1, need another `#` before comment blocks
xit('positions multiple comment blocks before each var assignment, not declaration', async () => {
const docUri = getDocUri('diagnostics/declaration-with-commentblock.coffee')
await testDiagnostics(docUri, [
{
range: sameLineRange(1, 0, 28),
severity: vscode.DiagnosticSeverity.Error,
message: "Type 'string' is not assignable to type 'number'."
}
])
[3, 0, 15],
[6, 0, 28],
[11, 1, 31],
[12, 1, 2],
[17, 2, 5],
[19, 1, 31],
[23, 1, 31],
[28, 1, 31],
[32, 1, 31],
[34, 1, 31],
[41, 1, 31],
[44, 0, 30]
].map(x => ({
range: sameLineRange(x[0], x[1], x[2]),
severity: vscode.DiagnosticSeverity.Error,
message: string_to_number_error
})), true)
})
})
2 changes: 1 addition & 1 deletion test/lsp/fixture/definition/basic.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ comprehension_6 = v * 2 for v in comprehension_5
item_1
item_2

``###*
###*
# @typedef {any} DefJSDoc1
###
###* @typedef {any} DefJSDoc2 ###
Expand Down
40 changes: 39 additions & 1 deletion test/lsp/fixture/diagnostics/declaration-with-commentblock.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,42 @@ diagnostics_ts2 = 1
diagnostics_ts2 = '1'

###* @type {number} ###
diagnostics_should_be_number = '123'
diagnostics_should_be_number = '123'


do =>
###* @type {number} ###
diagnostics_should_be_number_x = '1'
a =
###* @type {number} ###
diagnostics_should_be_number_b = '1'
c =
###* @type {number} ###
d: '1'
###* @type {number} ###
diagnostics_should_be_number_y = '1'
###*
# @type {number}
###
diagnostics_should_be_number_g = '1'
#
###*
# @type {number}
###
diagnostics_should_be_number_i = '1'
``###*
# @type {number}
###
diagnostics_should_be_number_j = '1'
``###* @type {number} ###
diagnostics_should_be_number_k = '1'
###
some.comment
###
###*
@type {number}
###
diagnostics_should_be_number_h = '1'
1
###* @type {number} ###
diagnostics_should_be_number_e = '2'

0 comments on commit cad5151

Please sign in to comment.