Merge pull request #227 from liuxy0551/fix_165

fix: #165 disable completion in comments

Author: Cythia828

src/languageFeatures.ts

@@ -145,6 +145,33 @@ function toDiagnostics(_resource: Uri, diag: ParseError): editor.IMarkerData {
 	};
 }
 
+interface TokenizationTextModel {
+	tokenization?: {
+		tokenizeLinesAt?: (
+			lineNumber: number,
+			lines: string[]
+		) => Array<{
+			findTokenIndexAtOffset: (offset: number) => number;
+			getStandardTokenType: (tokenIndex: number) => number;
+		}> | null;
+	};
+}
+
+function isPositionInComment(model: editor.IReadOnlyModel, position: Position): boolean {
+	const lineContent = model.getLineContent(position.lineNumber);
+	const tokenizationModel = model as editor.IReadOnlyModel & TokenizationTextModel;
+	// 在光标处追加哨兵字符，避免将刚结束的块注释误判为仍在注释中
+	const tokenizedLine = tokenizationModel.tokenization?.tokenizeLinesAt?.(position.lineNumber, [
+		`${lineContent.slice(0, position.column - 1)}x`
+	])?.[0];
+	if (!tokenizedLine) {
+		return false;
+	}
+
+	const tokenIndex = tokenizedLine.findTokenIndexAtOffset(position.column - 1);
+	return tokenizedLine.getStandardTokenType(tokenIndex) === 1;
+}
+
 export class CompletionAdapter<T extends BaseSQLWorker>
 	implements languages.CompletionItemProvider
 {
@@ -165,6 +192,10 @@ export class CompletionAdapter<T extends BaseSQLWorker>
 		context: languages.CompletionContext,
 		_token: CancellationToken
 	): Promise<languages.CompletionList> {
+		if (isPositionInComment(model, position)) {
+			return Promise.resolve({ suggestions: [] });
+		}
+
 		const resource = model.uri;
 		return this._worker(resource)
 			.then((worker) => {

src/test/languageFeatures.test.ts

@@ -0,0 +1,229 @@
+import * as assert from 'assert';
+
+import { BaseSQLWorker } from '../baseSQLWorker';
+import { LanguageIdEnum } from '../common/constants';
+import {
+	CancellationTokenSource,
+	editor,
+	languages,
+	Position
+} from '../fillers/monaco-editor-core';
+import { CompletionAdapter, WorkerAccessor } from '../languageFeatures';
+import { language as flinkLanguage } from '../languages/flink/flink';
+import { language as genericLanguage } from '../languages/generic/generic';
+import { language as hiveLanguage } from '../languages/hive/hive';
+import { language as impalaLanguage } from '../languages/impala/impala';
+import { language as mysqlLanguage } from '../languages/mysql/mysql';
+import { language as pgsqlLanguage } from '../languages/pgsql/pgsql';
+import { language as sparkLanguage } from '../languages/spark/spark';
+import { language as trinoLanguage } from '../languages/trino/trino';
+import { LanguageServiceDefaultsImpl, modeConfigurationDefault } from '../monaco.contribution';
+
+const SQL_DIALECTS = [
+	{ name: LanguageIdEnum.FLINK, language: flinkLanguage },
+	{ name: LanguageIdEnum.HIVE, language: hiveLanguage },
+	{ name: LanguageIdEnum.MYSQL, language: mysqlLanguage },
+	{ name: LanguageIdEnum.PG, language: pgsqlLanguage },
+	{ name: LanguageIdEnum.SPARK, language: sparkLanguage },
+	{ name: LanguageIdEnum.TRINO, language: trinoLanguage },
+	{ name: LanguageIdEnum.IMPALA, language: impalaLanguage },
+	{ name: LanguageIdEnum.GENERIC, language: genericLanguage }
+].map((dialect) => ({
+	...dialect,
+	languageId: `${dialect.name}-completion-test`
+}));
+const MYSQL_DIALECT = SQL_DIALECTS.find(({ name }) => name === LanguageIdEnum.MYSQL)!;
+
+SQL_DIALECTS.forEach((dialect) => {
+	languages.register({ id: dialect.languageId });
+	languages.setMonarchTokensProvider(dialect.languageId, dialect.language);
+});
+
+interface CompletionResult {
+	suggestions: languages.CompletionItem[];
+	workerCallCount: number;
+}
+
+function getEndPosition(value: string): Position {
+	const lines = value.split('\n');
+	return new Position(lines.length, lines[lines.length - 1].length + 1);
+}
+
+async function provideCompletionItems(
+	languageId: string,
+	value: string,
+	position: Position = getEndPosition(value)
+): Promise<CompletionResult> {
+	const model = editor.createModel(value, languageId);
+	const cancellationTokenSource = new CancellationTokenSource();
+	let workerCallCount = 0;
+	const worker: WorkerAccessor<BaseSQLWorker> = async () => {
+		workerCallCount++;
+		return {
+			doCompletionWithEntities: async () => ({
+				suggestions: {
+					syntax: [],
+					keywords: ['SELECT']
+				},
+				allEntities: null,
+				context: null
+			})
+		} as unknown as BaseSQLWorker;
+	};
+	const defaults = new LanguageServiceDefaultsImpl(languageId, modeConfigurationDefault);
+	const adapter = new CompletionAdapter(worker, defaults);
+
+	try {
+		const completionList = await adapter.provideCompletionItems(
+			model,
+			position,
+			{ triggerKind: languages.CompletionTriggerKind.Invoke },
+			cancellationTokenSource.token
+		);
+
+		return {
+			suggestions: completionList.suggestions,
+			workerCallCount
+		};
+	} finally {
+		cancellationTokenSource.dispose();
+		model.dispose();
+	}
+}
+
+SQL_DIALECTS.forEach((dialect) => {
+	test(`does not provide ${dialect.name} completion items inside a line comment`, async () => {
+		const result = await provideCompletionItems(dialect.languageId, 'SELECT 1 -- comment');
+
+		assert.deepStrictEqual(result.suggestions, []);
+		assert.strictEqual(result.workerCallCount, 0);
+	});
+
+	test(`does not provide ${dialect.name} completion items inside a multiline block comment`, async () => {
+		const result = await provideCompletionItems(
+			dialect.languageId,
+			'SELECT /* comment\nstill comment */',
+			new Position(2, 6)
+		);
+
+		assert.deepStrictEqual(result.suggestions, []);
+		assert.strictEqual(result.workerCallCount, 0);
+	});
+});
+
+test('does not provide completion items after a line comment marker', async () => {
+	const result = await provideCompletionItems(MYSQL_DIALECT.languageId, '--');
+
+	assert.deepStrictEqual(result.suggestions, []);
+	assert.strictEqual(result.workerCallCount, 0);
+});
+
+test('does not provide completion items inside a MySQL hash comment', async () => {
+	const result = await provideCompletionItems(MYSQL_DIALECT.languageId, '# comment');
+
+	assert.deepStrictEqual(result.suggestions, []);
+	assert.strictEqual(result.workerCallCount, 0);
+});
+
+test('does not provide completion items inside a block comment', async () => {
+	const result = await provideCompletionItems(
+		MYSQL_DIALECT.languageId,
+		'/* comment */',
+		new Position(1, 4)
+	);
+
+	assert.deepStrictEqual(result.suggestions, []);
+	assert.strictEqual(result.workerCallCount, 0);
+});
+
+test('provides completion items after a closed block comment', async () => {
+	const result = await provideCompletionItems(MYSQL_DIALECT.languageId, 'SELECT /* comment */');
+
+	assert.deepStrictEqual(
+		result.suggestions.map((item) => item.label),
+		['SELECT']
+	);
+	assert.strictEqual(result.workerCallCount, 1);
+});
+
+test('does not treat comment markers inside strings as comments', async () => {
+	const result = await provideCompletionItems(MYSQL_DIALECT.languageId, "SELECT '--'");
+
+	assert.deepStrictEqual(
+		result.suggestions.map((item) => item.label),
+		['SELECT']
+	);
+	assert.strictEqual(result.workerCallCount, 1);
+});
+
+test('does not treat a single minus sign as a comment', async () => {
+	const result = await provideCompletionItems(MYSQL_DIALECT.languageId, '-');
+
+	assert.deepStrictEqual(
+		result.suggestions.map((item) => item.label),
+		['SELECT']
+	);
+	assert.strictEqual(result.workerCallCount, 1);
+});
+
+test('checks comment state without reading the whole document prefix', async () => {
+	const value = 'SELECT 1;\n-- comment';
+	const model = editor.createModel(value, MYSQL_DIALECT.languageId);
+	const position = getEndPosition(value);
+	const originalGetValueInRange = model.getValueInRange.bind(model);
+	const cancellationTokenSource = new CancellationTokenSource();
+	const defaults = new LanguageServiceDefaultsImpl(
+		MYSQL_DIALECT.languageId,
+		modeConfigurationDefault
+	);
+	const adapter = new CompletionAdapter(
+		async () =>
+			({
+				doCompletionWithEntities: async () => ({
+					suggestions: {
+						syntax: [],
+						keywords: ['SELECT']
+					},
+					allEntities: null,
+					context: null
+				})
+			}) as unknown as BaseSQLWorker,
+		defaults
+	);
+
+	model.getValueInRange = ((range, eol) => {
+		if (
+			range.startLineNumber === 1 &&
+			range.startColumn === 1 &&
+			(range.endLineNumber > 1 || range.endColumn > 1)
+		) {
+			throw new Error('comment detection should not read the whole document prefix');
+		}
+
+		return originalGetValueInRange(range, eol);
+	}) as typeof model.getValueInRange;
+
+	try {
+		const completionList = await adapter.provideCompletionItems(
+			model,
+			position,
+			{ triggerKind: languages.CompletionTriggerKind.Invoke },
+			cancellationTokenSource.token
+		);
+
+		assert.deepStrictEqual(completionList.suggestions, []);
+	} finally {
+		cancellationTokenSource.dispose();
+		model.dispose();
+	}
+});
+
+test('keeps the existing completion flow for regular SQL', async () => {
+	const result = await provideCompletionItems(MYSQL_DIALECT.languageId, 'SELECT ');
+
+	assert.deepStrictEqual(
+		result.suggestions.map((item) => item.label),
+		['SELECT']
+	);
+	assert.strictEqual(result.workerCallCount, 1);
+});

test/all.js

@@ -103,7 +103,7 @@ requirejs(
 	function () {
 		let files;
 		try {
-			files = glob.sync('out/amd/languages/*/*.test.js', {
+			files = glob.sync(['out/amd/languages/*/*.test.js', 'out/amd/test/*.test.js'], {
 				cwd: path.dirname(__dirname),
 				dot: true
 			});

test/setup.js

@@ -29,6 +29,17 @@ define('vs/nls', [], {
 	}
 });
 
+define('dt-sql-parser', [], {
+	EntityContextType: {
+		TABLE: 'table',
+		TABLE_CREATE: 'tableCreate'
+	}
+});
+
+define('monaco-editor', ['vs/editor/editor.main'], function (api) {
+	return api.m || api;
+});
+
 define(['vs/editor/editor.main'], function (api) {
 	// Monaco Editor 0.54.0+ exports as api.m instead of api directly
 	const monaco = api.m || api;