Skip to content

Commit

Permalink
fix autocomplete for inline object keys in more specialized condition…
Browse files Browse the repository at this point in the history 
…s such as inline after other key-values

internal trailing space transforms revise
  • Loading branch information
@phil294
phil294 committed May 20, 2022
1 parent 8676ec0 commit 6cc76e4
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 61 deletions.
3 changes: 2 additions & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ There is lot of hacky code to get this all to work. One thing to keep in mind is
- Annotating constructor `@parameters` with JSDoc can not provide type hints when you use a variable with the same name outside ([issue](https://github.com/phil294/coffeesense/issues/5)). This will be fixed at some point.
- General:
- Make sure you never leave any dangling indentation in your source code around, unless it's the line you are working on. In VSCode, this is the default - just make sure to **not** override `"editor.trimAutoWhitespace"` to `false`. Keep it at its default `true`. Same thing goes for other IDEs: Try not to have more than one empty line with indentation. This is because CoffeeSense treats any line with indent as a possible place for you to define new object properties or arguments, as it is not aware of the cursor position while compiling. It injects certain characters at these lines which gets messy if you're on another line.
- Trailing whitespace is disallowed because it takes on special meaning while autocompleting
- Avoid trailing whitespace because it takes on special meaning
- Autocompletion is optimized for the common code style of single space spacing. E.g. it is better to write `a = b: c` instead of `a=b:c` as the test cases simply do not cover the latter.
- Cosmetics:
- JSDoc lines with trailing space or dot can look funny in tooltips if you don't start the line with a number sign ([issue](https://github.com/phil294/coffeesense/issues/11)).

Expand Down
24 changes: 17 additions & 7 deletions server/src/modes/script/javascript.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { LanguageMode } from '../../embeddedSupport/languageModes';
import { LANGUAGE_ID } from '../../language';
import { DependencyService, RuntimeLibrary } from '../../services/dependencyService';
import { EnvironmentService } from '../../services/EnvironmentService';
import transpile_service, { common_js_variable_name_character, trailing_param_regex, get_line_at_line_no, get_word_around_position, pseudo_compile_coffee } from '../../services/transpileService';
import transpile_service, { common_js_variable_name_character, get_line_at_line_no, get_word_around_position, pseudo_compile_coffee } from '../../services/transpileService';
import { IServiceHost } from '../../services/typescriptService/serviceHost';
import { toCompletionItemKind, toSymbolKind } from '../../services/typescriptService/util';
import { CodeActionData, CodeActionDataKind, OrganizeImportsActionData } from '../../types';
Expand Down Expand Up @@ -226,6 +226,8 @@ export async function getJavascriptMode(
// source maps in between import modules names, so when adding a new module, move cursor
// to start of first existing import which gives correct results:
coffee_position_tweaked.character = 9
} else if(coffee_next_char === '}' && coffee_last_char === ' ' && coffee_second_last_char === ',') {
coffee_position_tweaked.character--
}
let js_position = transpile_service.position_coffee_to_js(transpilation, coffee_position_tweaked, coffee_doc)
if(!js_position) {
Expand Down Expand Up @@ -275,6 +277,9 @@ export async function getJavascriptMode(
if(js_second_last_char === ':' && js_last_char === ' ' && js_next_char?.match(common_js_variable_name_character)) {
// source maps are wrong, cursor is in CS before : but in JS after :, fix this:
char_offset = -2
} else if(js_next_char === '{') {
// pretty much always want to know what's *inside* the object, when sourcemap points directly before
char_offset++
} else {
const special_trigger_chars = ['"', "'"]
for(const s of special_trigger_chars) {
Expand Down Expand Up @@ -310,13 +315,18 @@ export async function getJavascriptMode(
}
let completions = service.getCompletionsAtPosition(fileFsPath, js_offset, completion_options);

if(coffee_line.match(trailing_param_regex)) {
if(js_next_char === '}' && js_last_char === '{') {
// Special case: `{}` was inserted by transpileService to get items for *possible* object param keys.
// However, it must also be possible to insert normal variable references here - at this point, it is
if(! js_line.startsWith('import') && ! js_line.includes('require(') && coffee_last_char !== '{' && coffee_second_last_char !== '{') {
let outside_brace_check_offset = 0
if(js_next_char === '{')
// offset is ++, so we're actually *after* the { now
outside_brace_check_offset = -1
else if(js_next_char === '}')
outside_brace_check_offset = 1
if(outside_brace_check_offset) {
// It must also be possible to insert normal variable references here - at this point, it is
// impossible to predict whether the user wants to define a new object here or insert a var.
// Get suggestions for both:
const completions_outside_object = service.getCompletionsAtPosition(fileFsPath, js_offset - 1, completion_options);
// Get suggestions for both by moving the cursor outside the object:
const completions_outside_object = service.getCompletionsAtPosition(fileFsPath, js_offset + outside_brace_check_offset, completion_options);
if(completions_outside_object) {
if(completions) {
completions.entries.forEach(e => e.sortText = '0') // prefer inside to outside
Expand Down
84 changes: 39 additions & 45 deletions server/src/services/transpileService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,8 @@ interface ITranspileService {
const MD5 = new jshashes.MD5()
const transpilation_cache: Map<string,ITranspilationResult> = new VolatileMap(180000)

export const trailing_param_regex = /^[^#]*[^ #] $/mg

/** The resulting coffee must still always be valid and parsable by the compiler */
/** 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()
// Enable autocomplete at `@|`. Replace with magic snippet that allows for both @|
Expand Down Expand Up @@ -90,19 +89,23 @@ function preprocess_coffee(coffee_doc: TextDocument) {
logger.logDebug(`transform ^### to ^\`\`### ${coffee_doc.uri}`)
return ws + '``###' + c
})
// Trailing spaces = `↯:↯`. Something to make the line not error, and object to get autocomplete with params
// inline object keys, both as new obj param and as new entry to an existing object param.
// These characters are again removed in postprocess_js and gets special handling in doComplete().
// Trailing open brace on the other hand cannot be replaced because it is valid CS (s.a. signature hint tests).
.replaceAll(/^[^#\n]*[^ #\n] $/mg, (m) => {
logger.logDebug(`replace trailing space with ↯:↯ ${coffee_doc.uri}`)
return m + '↯:↯'
})
// Similar inside objects: `{ ..., }` -> add another element to it after comma.
// Shorthand not like above to keep line length.
.replaceAll(/, (\s*)\}/mg, (_, ws) => {
logger.logDebug(`replace trailing comma inside { } with ↯ ${coffee_doc.uri}`)
return `,↯${ws}}`
})
const tmp_lines = tmp.split('\n')
const object_tweak_coffee_lines: number[] = []
const space_tweak_coffee_lines: { line: string, line_i: number }[] = []
tmp_lines.forEach((line, line_i) => {
if(line.match(trailing_param_regex)) {
// Trailing spaces = `{𝆮}`. Something to make the line not error, and `{}` to get autocomplete with params inline object keys.
// Handled in in postprocess_js and gets special handling in doComplete().
// Trailing open brace on the other hand cannot be replaced because it is valid CS (s.a. signature hint tests).
logger.logDebug(`replace trailing space with {𝆮} ${coffee_doc.uri}`)
space_tweak_coffee_lines.push({ line_i, line })
tmp_lines[line_i] = `${line}{𝆮}`
return
}
// Enable autocomplete on empty lines inside object properties.
// Normally, empty lines get deleted by the cs compiler and cannot be mapped back. Insert some
// random unicode snippet to keep the lines, and remove these snippets right after compilation below,
Expand All @@ -127,7 +130,7 @@ function preprocess_coffee(coffee_doc: TextDocument) {
}
})
const coffee = tmp_lines.join('\n')
return { coffee, object_tweak_coffee_lines, space_tweak_coffee_lines }
return { coffee, 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 @@ -379,7 +382,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[], space_tweak_coffee_lines: { line: string, line_i: number }[]) {
function postprocess_js(result: ITranspilationResult, object_tweak_coffee_lines: number[]) {
if(!result.js || !result.source_map)
return

Expand All @@ -389,34 +392,14 @@ function postprocess_js(result: ITranspilationResult, object_tweak_coffee_lines:
`${asynk || ''}${asterisk}${func_name} (`)
// see preprocess
.replaceAll('this.____CoffeeSenseAtSign', '(this.valueOf(),this) ')
// coffee `↯:↯` *always* results in a separate line in js so no source mapping is required, just rm them
.replaceAll(/↯(: )?/g, (m) =>
' '.repeat(m.length))

const js_lines = result.js.split('\n')

result.source_map.forEach(source_map_line => {
source_map_line.columns.forEach(source_map_entry => {
// See usage of 𝆮 above
for(const space_tweak_coffee_line of space_tweak_coffee_lines) {
if(source_map_entry.sourceLine === space_tweak_coffee_line.line_i) {
const js_line = js_lines[source_map_entry.line]!
const invocation_match = js_line.match(/\(\{𝆮\}\);$/)
if(invocation_match) {
js_lines[source_map_entry.line] = js_line.slice(0, invocation_match.index) + '({} '
source_map_entry.sourceColumn = space_tweak_coffee_line.line.length
source_map_entry.column = invocation_match.index! + 2
} else {
const comma_param_match = js_line.match(/, \{𝆮\}\);$/)
if(comma_param_match) {
js_lines[source_map_entry.line] = js_line.slice(0, comma_param_match.index) + ', {}) '
source_map_entry.sourceColumn = space_tweak_coffee_line.line.length
source_map_entry.column = comma_param_match.index! + 3
} else {
// Don't completely remove other mappings for this line so maybe go-tos might still work,
// but they can't take precedence over the match at the end of the line due to no word match
source_map_entry.sourceColumn = 0
}
}
}
}
// See usage of 𒐛 above
for(const obj_tweak_coffee_line of object_tweak_coffee_lines) {
// This logic is equivalent to fake line source map fixing, explained there
Expand Down Expand Up @@ -577,7 +560,7 @@ const transpile_service: ITranspileService = {
return cached
}

const { coffee: preprocessed_coffee, object_tweak_coffee_lines, space_tweak_coffee_lines } = preprocess_coffee(orig_coffee_doc)
const { coffee: preprocessed_coffee, object_tweak_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 @@ -590,7 +573,7 @@ const transpile_service: ITranspileService = {
}

if(result.js && result.source_map) {
postprocess_js(result, object_tweak_coffee_lines, space_tweak_coffee_lines)
postprocess_js(result, object_tweak_coffee_lines)
} else {
// Nothing worked at all. As a last resort, just pass the coffee to tsserver,
// with minimal transforms:
Expand Down Expand Up @@ -700,6 +683,16 @@ const transpile_service: ITranspileService = {
if(js_matches_by_cut_off_chars.length)
return js_matches_by_cut_off_chars
}
let prev = '', i = coffee_position_offset - 1
while((prev = coffee_doc.getText()[i]||' ') === ' ')
i--
if(prev === '{') {
// e.g. wrong source map cs `|{}` to js `{|}`
const js_matches_by_previous_brace = js_matches_by_line
.filter(c => c?.sourceColumn === coffee_position.character - coffee_position_offset + i)
if(js_matches_by_previous_brace.length)
return js_matches_by_previous_brace
}
return js_matches_by_line
}
const js_matches = get_fitting_js_matches()
Expand Down Expand Up @@ -740,10 +733,11 @@ const transpile_service: ITranspileService = {
if(js_matches_by_line_contains_word.length)
return abcdefg(js_matches_by_line_contains_word)
}
const js_matches_by_line_contains_any_common_char = js_matches.filter(match =>
get_line_at_line_no(js_doc_tmp, match.line).match(common_js_variable_name_character))
if(js_matches_by_line_contains_any_common_char.length)
return abcdefg(js_matches_by_line_contains_any_common_char)
// Doesn't make much sense here as match_by_char in abcdefg comes after...
// const js_matches_by_line_contains_any_common_char = js_matches.filter(match =>
// get_line_at_line_no(js_doc_tmp, match.line).match(common_js_variable_name_character))
// if(js_matches_by_line_contains_any_common_char.length)
// return abcdefg(js_matches_by_line_contains_any_common_char)
return abcdefg(js_matches)
}
const js_match = choose_js_match()
Expand All @@ -770,7 +764,7 @@ const transpile_service: ITranspileService = {
column += (coffee_line_until_cursor.split('@').length - 1) * ('this.'.length - '@'.length)
}

// logger.logDebug(`mapped CS => JS: ${coffee_position.line}:${coffee_position.character} => ${match?.line}:${match?.column}`)
logger.logDebug(`mapped CS => JS: ${coffee_position.line}:${coffee_position.character} => ${line}:${column}`)
if(line == null || column == null)
return undefined
return Position.create(line, column)
Expand Down
11 changes: 7 additions & 4 deletions test/completionHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export async function testCompletion({ doc_uri, position, expected_items: expect
position
)) as vscode.CompletionList;

// todo allow_unspecified is not in use??
if(!allow_unspecified && !allow_globals)
//@ts-ignore
assert.equal(expectedItems.length, result.items.filter(i => i.label.label !== '#region' && i.label.label !== '#endregion' && i.label !== '#region' && i.label !== '#endregion').length)
Expand All @@ -54,22 +55,24 @@ export async function testCompletion({ doc_uri, position, expected_items: expect
assert.ok(! result.items.some(i => unexpected_items.includes(i.label.label? i.label.label : i.label)))

expectedItems.forEach(ei => {
let match_index = -1
if (typeof ei === 'string') {
assert.ok(
result.items.some(i => {
match_index = result.items.findIndex(i => {
return i.label === ei &&
// Omit standard matches like variable as these primarily yield false positives.
// If these are really required, they can be passed separately.
[CompletionItemKind.Function, CompletionItemKind.Property, CompletionItemKind.Field].includes(i.kind || -1)
}),
})
assert.ok(match_index > -1,
`Can't find matching item for\n${JSON.stringify(ei, null, 2)}\nSeen items:\n${JSON.stringify(
result.items,
null,
2
)}`
);
} else {
const match = matchFn ? result.items.find(matchFn(ei)) : result.items.find(i => i.label === ei.label);
const match_index = matchFn ? result.items.findIndex(matchFn(ei)) : result.items.findIndex(i => i.label === ei.label);
const match = result.items[match_index]
if (!match) {
assert.fail(
`Can't find matching item for\n${JSON.stringify(ei, null, 2)}\nSeen items:\n${JSON.stringify(
Expand Down
32 changes: 30 additions & 2 deletions test/lsp/features/completion/basic.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ describe('Should autocomplete', () => {
const inline_object_param_key_uri = getDocUri('completion/inline-object-param-key.coffee')
await testCompletion({
doc_uri: inline_object_param_key_uri,
position: position(14, 28),
position: position(13, 28),
expected_items: [
'obj_inline_param_key_prop_1', // TODO expect pos 1? as there are globals. perhaps always as part of testcompletion
{ label: 'obj_inline_param_key_unrelated_var', kind: CompletionItemKind.Variable }
Expand All @@ -200,7 +200,7 @@ describe('Should autocomplete', () => {
})
await testCompletion({
doc_uri: inline_object_param_key_uri,
position: position(16, 32),
position: position(15, 32),
expected_items: [
'obj_inline_param_key_prop_2',
{ label: 'obj_inline_param_key_unrelated_var', kind: CompletionItemKind.Variable }
Expand All @@ -211,6 +211,34 @@ describe('Should autocomplete', () => {
]
})
})
it('completes inline object (implicit) property keys in various scenarios', async () => {
const inline_object_param_key_uri = getDocUri('completion/inline-object-param-key.coffee')
const checks = [
[30, 5, 'oi2'], [30, 17, 'oi2'], [30, 18, 'oi2'], [30, 21, ['oi3', 'oi4'], true], [30, 22, ['oi3', 'oi4']],
[31, 22, ['oi3', 'oi4']], [31, 23, ['oi3', 'oi4']], [31, 24, ['oi3', 'oi4'], true], [31, 25, ['oi3', 'oi4'], true], [31, 26, ['oi3', 'oi4'], true],
[32, 22, 'oi4'], [32, 23, ['oi3', 'oi4']], [32, 34, 'oi4'], [32, 35, 'oi4'],
[33, 21, ['oi3', 'oi4'], true],
[34, 5, 'oi2'], [34, 18, 'oi2'],
[35, 20, ['oi3', 'oi4'], true],
[36, 16, 'oi2'],
[37, 18, ['oi3', 'oi4'], true],
[39, 4, ['oi3', 'oi4']],
[40, 11, ['oi7', 'oi8'], true],
[42, 21, 'oi2'],
[43, 8, ['oi1', 'oi2'], true],
[45, 7, ['oi11', 'oi12'], true],
[47, 19, 'oi15'],
] as const
for(const check of checks) {
await testCompletion({
doc_uri: inline_object_param_key_uri,
position: position(check[0], check[1]),
//@ts-ignore
expected_items: Array.isArray(check[2]) ? check[2] : [ check[2] ],
allow_globals: check[3] || false
})
}
})

it('completes inline object property keys as function params even without a colon, after opened but not yet closed brace', async () => {
await testCompletion({ doc_uri: getDocUri('completion/inline-object-open-brace.coffee'), position: position(9, 52), expected_items: ['inline_obj_open_brace_prop_1'] })
Expand Down
33 changes: 31 additions & 2 deletions test/lsp/fixture/completion/inline-object-param-key.coffee
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

###*
# @param one {{
# obj_inline_param_key_prop_1: number
Expand All @@ -16,4 +15,34 @@ obj_inline_param_key_method
#
obj_inline_param_key_method {},
#
#
#

###*
# @param {{ oi1: string, oi2: string }} _
# @param {{ oi3: string, oi4: string }} _
###
oi5 = ({ oi1, oi2 }, { oi3, oi4 }) =>
###*
# @param {string} oi6
# @param {{ oi7: string, oi8: string }} _
###
oi9 = (oi6, { oi7, oi8 }) =>

oi5 { oi1: 'op1', }, {}
oi5 { oi1: 'op1', }, { }
oi5 { oi1: 'op2', }, { oi3: 'op7', }
oi5 { oi1: 'op3', },
oi5 { oi1: 'op3',
oi5 { oi1: 'op4' },
oi5 oi1: 'op9',
oi5 (oi1: 'op5'),
oi5 (oi1: 'op6'),

oi9 'op8',
do =>
oi5 oi1: 'op10',
oi5
###* @type {{ oi11: string, oi12: string }} ###
oi13 =
###* @type {{ oi14: string, oi15: string }} ###
oi15 = oi14: '123',

0 comments on commit 6cc76e4

Please sign in to comment.