Skip to content

Commit

Permalink
Revert "improve automatic type detection at variable assignment time"
Browse files Browse the repository at this point in the history
As it does not work with block commented variables yet. Now it's the hacky solution again. Will go back once again once this also works.

This reverts commit c71f458.
  • Loading branch information
phil294 committed Feb 28, 2022
1 parent cc1efec commit 0a147d1
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 5 deletions.
2 changes: 1 addition & 1 deletion server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"vscode-languageserver": "7.0.0",
"vscode-languageserver-textdocument": "^1.0.1",
"vscode-uri": "^3.0.2",
"coffeescript": "github:edemaine/coffeescript#var-assign",
"coffeescript": "^2.5.1",
"jshashes": "^1.0.8",
"typescript": "^4.3.2",
"volatile-map": "^1.0.2"
Expand Down
127 changes: 127 additions & 0 deletions server/src/services/transpileService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,133 @@ function postprocess_js(result: ITranspilationResult, object_tweak_coffee_lines:
}
}))
}

// console.time('var-decl-fix')
//////////////////////////////////////
///////// Modify variable declarations to solve various TS compiler errors:
// Should not be error but is:
// xy = 123 # Error: Variable 'xy' implicitly has type 'any' in some locations where its type cannot be determined.CoffeeSense [TS](7034)
// => xy # Error: Variable 'xy' implicitly has an 'any' type.CoffeeSense [TS](7005)
//////// and
// Should be error but is not:
// a = 1
// a = 'one'
/////// This is because the cs compiler puts variable declarations to the front:
// Translates to:
// var a;
// a = 1;
// a = 'one';
/////// 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:
/*
var a, b, c;
a = 1;
[b] = 2;
({c} = 3);
*/
// Shall become:
/*
var c;
let a = 1; // added let
let [b] = 2; // added let
({c} = 3); // unchanged because of surrounding braces
*/
// similarly, array destructors with more than one variable cannot be changed.
// Returns stay untouched (return x = 1) too.
const js_line_nos = Array.from(Array(js_lines.length).keys())
// Part 1: Determine declaration areas (` var x, y;`)
const js_decl_lines_info = js_line_nos
.map(decl_line_no => {
const match = js_lines[decl_line_no]!.match(/^(\s*)(var )(.+);$/)
if(match) {
const var_decl_infos = match[3]!.split(', ').map(var_name => ({
var_name,
decl_indent: match[1]!.length,
decl_line_no,
}))
return {
decl_line_no,
var_decl_infos,
}
}
return null
})
.filter(Boolean)
// Part 2: For each `var` decl, find fitting first impl statement
// (`x = 1`), if present, and return new line content (`let x = 1`).
// Might as well be `var x = 1` but this helps differentiating/debugging
const js_impl_line_changes = js_decl_lines_info
.map(info => info!.var_decl_infos)
.flat()
.map(({ var_name, decl_indent, decl_line_no }) => {
const js_line_nos_after_decl = js_line_nos.slice(decl_line_no)
for(const impl_line_no of js_line_nos_after_decl) {
const line = js_lines[impl_line_no]!
const impl_whitespace = line.match(/^\s*/)![0]!
const impl_indent = impl_whitespace.length
if(impl_indent < decl_indent)
// Parent block scope. Need to skip this variable then, no impl has been found
// before current block got closed. It is important to stop here, as otherwise
// it might later match an impl from *another* decl of the same var name
return null
const var_impl_text = `${var_name} = `
if(line.substr(impl_indent, var_impl_text.length) === var_impl_text) {
if(impl_indent > decl_indent)
// This is a conditional first value assignment and type can not safely be set
return null
const rest_of_line = line.slice(impl_indent + var_impl_text.length)
return {
var_name,
impl_line_no,
decl_line_no,
new_line_content: `${impl_whitespace}let ${var_impl_text}${rest_of_line}`,
new_let_column: impl_indent,
}
}
}
return null
}).filter(Boolean)
// Part 3: Apply Part 2 changes and update source maps of those lines
for(const change of js_impl_line_changes) {
js_lines[change!.impl_line_no] = change!.new_line_content
const map_columns = result.source_map[change!.impl_line_no]!.columns
const map_current_impl_start = map_columns[change!.new_let_column]!
// Can be null in cases where the variable is not user-set but e.g. a helper
// variable put there by the cs compiler itself and ignored otherwise
if(map_current_impl_start != null) {
map_columns.splice(
change!.new_let_column,
0,
..."let ".split('').map((_, i) => ({
...map_current_impl_start,
column: map_current_impl_start.column + i
})))
for(let i = map_current_impl_start.column + "let ".length; i < map_columns.length + "let ".length; i++) {
if(map_columns[i])
map_columns[i]!.column += "let ".length // or = i
}
}
}
// Part 4: Update decl lines (Part 1). Where no impl lines were found (Part 2),
// keep them. If all were modified, an empty line will be put.
for(const decl_line_info of js_decl_lines_info) {
let new_decl_line = decl_line_info!.var_decl_infos
.filter(decl_info => ! js_impl_line_changes.some(impl_change =>
impl_change!.var_name === decl_info.var_name &&
impl_change!.decl_line_no === decl_info.decl_line_no))
.map(i => i.var_name)
.join(', ')
if(new_decl_line)
// Those that could not be changed
new_decl_line = 'var ' + new_decl_line
js_lines[decl_line_info!.decl_line_no] = new_decl_line
}

result.js = js_lines.join('\n')
// console.timeEnd('var-decl-fix')

result.js = result.js
// See usage of 𒐛 above
// Note that outside of objects, this will leave empty objects behind
Expand Down
6 changes: 3 additions & 3 deletions test/lsp/features/diagnostics/basic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ describe('Should find diagnostics', () => {
])
})

// Will be fixed some time, see https://github.com/jashkenas/coffeescript/issues/5366#issuecomment-1021366654. Could go back to previous version in the meantime where the workaround for declaration typing was still in place
xit('pushes down variable declaration to assignment even with comment block before it', async () => {
// TODO: go back to new CS branch, https://github.com/jashkenas/coffeescript/issues/5366#issuecomment-1021366654
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, 14),
range: sameLineRange(3, 0, 15),
severity: vscode.DiagnosticSeverity.Error,
message: "Type 'string' is not assignable to type 'number'."
}
Expand Down
2 changes: 1 addition & 1 deletion test/lsp/features/hover/basic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ describe('Should do hover', () => {

it('shows inherited type from another variable', async () => {
await testHover(doc_uri, position(1, 0), {
contents: ['\n```ts\nvar hover2: number\n```'],
contents: ['\n```ts\nlet hover2: number\n```'],
range: sameLineRange(1, 0, 0)
})
})
Expand Down

0 comments on commit 0a147d1

Please sign in to comment.