Merge pull request #38924 from tsalinger/fix38623

Fix #38623

src/vs/editor/contrib/quickFix/quickFix.ts

@@ -13,6 +13,7 @@ import { TPromise } from 'vs/base/common/winjs.base';
 import { onUnexpectedExternalError, illegalArgument } from 'vs/base/common/errors';
 import { IModelService } from 'vs/editor/common/services/modelService';
 import { registerLanguageCommand } from 'vs/editor/browser/editorExtensions';
+import { isFalsyOrEmpty } from 'vs/base/common/arrays';
 
 export function getCodeActions(model: IReadOnlyModel, range: Range): TPromise<CodeAction[]> {
 
@@ -31,7 +32,26 @@ export function getCodeActions(model: IReadOnlyModel, range: Range): TPromise<Co
 		});
 	});
 
-	return TPromise.join(promises).then(() => allResults);
+	return TPromise.join(promises).then(
+		() => allResults.sort(codeActionsComparator)
+	);
+}
+
+function codeActionsComparator(a: CodeAction, b: CodeAction): number {
+
+	const aHasDiags = !isFalsyOrEmpty(a.diagnostics);
+	const bHasDiags = !isFalsyOrEmpty(b.diagnostics);
+	if (aHasDiags) {
+		if (bHasDiags) {
+			return a.diagnostics[0].message.localeCompare(b.diagnostics[0].message);
+		} else {
+			return -1;
+		}
+	} else if (bHasDiags) {
+		return 1;
+	} else {
+		return 0;	// both have no diagnostics
+	}
 }
 
 registerLanguageCommand('_executeCodeActionProvider', function (accessor, args) {

src/vs/editor/contrib/quickFix/test/quickFix.test.ts

@@ -8,7 +8,7 @@ import * as assert from 'assert';
 import URI from 'vs/base/common/uri';
 import Severity from 'vs/base/common/severity';
 import { Model } from 'vs/editor/common/model/model';
-import { CodeActionProviderRegistry, LanguageIdentifier, CodeActionProvider } from 'vs/editor/common/modes';
+import { CodeActionProviderRegistry, LanguageIdentifier, CodeActionProvider, Command, WorkspaceEdit, IResourceEdit } from 'vs/editor/common/modes';
 import { IDisposable, dispose } from 'vs/base/common/lifecycle';
 import { Range } from 'vs/editor/common/core/range';
 import { getCodeActions } from 'vs/editor/contrib/quickFix/quickFix';
@@ -19,6 +19,64 @@ suite('QuickFix', () => {
 	let uri = URI.parse('untitled:path');
 	let model: Model;
 	let disposables: IDisposable[] = [];
+	let testData = {
+		diagnostics: {
+			abc: {
+				title: 'bTitle',
+				diagnostics: [{
+					startLineNumber: 1,
+					startColumn: 1,
+					endLineNumber: 2,
+					endColumn: 1,
+					severity: Severity.Error,
+					message: 'abc'
+				}]
+			},
+			bcd: {
+				title: 'aTitle',
+				diagnostics: [{
+					startLineNumber: 1,
+					startColumn: 1,
+					endLineNumber: 2,
+					endColumn: 1,
+					severity: Severity.Error,
+					message: 'bcd'
+				}]
+			}
+		},
+		command: {
+			abc: {
+				command: new class implements Command {
+					id: '1';
+					title: 'abc';
+				},
+				title: 'Extract to inner function in function "test"'
+			}
+		},
+		spelling: {
+			bcd: {
+				diagnostics: [],
+				edits: new class implements WorkspaceEdit {
+					edits: IResourceEdit[];
+				},
+				title: 'abc'
+			}
+		},
+		tsLint: {
+			abc: {
+				$ident: 57,
+				arguments: [],
+				id: '_internal_command_delegation',
+				title: 'abc'
+			},
+			bcd: {
+				$ident: 47,
+				arguments: [],
+				id: '_internal_command_delegation',
+				title: 'bcd'
+			}
+		}
+	};
 
 	setup(function () {
 		model = Model.createFromString('test1\ntest2\ntest3', undefined, langId, uri);
@@ -29,30 +87,37 @@ suite('QuickFix', () => {
 		dispose(disposables);
 	});
 
-	test('basics', async function () {
+	test('CodeActions are sorted by type, #38623', async function () {
 
 		const provider = new class implements CodeActionProvider {
 			provideCodeActions() {
-				return [{
-					title: 'Testing1',
-					diagnostics: [{
-						startLineNumber: 1,
-						startColumn: 1,
-						endLineNumber: 2,
-						endColumn: 1,
-						severity: Severity.Error,
-						message: 'some error'
-					}]
-				}, {
-					title: 'Testing2'
-				}];
+				return [
+					testData.command.abc,
+					testData.diagnostics.bcd,
+					testData.spelling.bcd,
+					testData.tsLint.bcd,
+					testData.tsLint.abc,
+					testData.diagnostics.abc
+				];
 			}
 		};
 
 		disposables.push(CodeActionProviderRegistry.register('fooLang', provider));
 
+		const expected = [
+			// CodeActions with a diagnostics array are shown first ordered by diagnostics.message
+			testData.diagnostics.abc,
+			testData.diagnostics.bcd,
+
+			// CodeActions without diagnostics are shown in the given order without any further sorting
+			testData.command.abc,
+			testData.spelling.bcd, // empty diagnostics array
+			testData.tsLint.bcd,
+			testData.tsLint.abc
+		];
+
 		const actions = await getCodeActions(model, new Range(1, 1, 2, 1));
-		assert.equal(actions.length, 2);
+		assert.equal(actions.length, 6);
+		assert.deepEqual(actions, expected);
 	});
-
 });