fix positioning of comment blocks

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

Author: phil294

server/src/services/transpileService.ts

@@ -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.
@@ -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.
@@ -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 */
@@ -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')
@@ -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
 
@@ -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:
@@ -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:
@@ -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)
@@ -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:
@@ -657,7 +698,7 @@ const transpile_service: ITranspileService = {
         line_i++
       }
     }
-    
+
     if(mapped)
       coffee_pos = Position.create(mapped.sourceLine, mapped.sourceColumn)
     else
@@ -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!
@@ -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') {

test/diagnosticHelper.ts

@@ -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(

test/lsp/features/diagnostics/basic.test.ts

@@ -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 () => {
@@ -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),
@@ -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)
 	})
 })

test/lsp/fixture/definition/basic.coffee

@@ -19,7 +19,7 @@ comprehension_6 = v * 2 for v in comprehension_5
 item_1
 item_2
 
-``###*
+###*
 # @typedef {any} DefJSDoc1
 ###
 ###* @typedef {any} DefJSDoc2 ###

test/lsp/fixture/diagnostics/declaration-with-commentblock.coffee

@@ -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'