Surfacing better error messages about SQLCMD var names (#22509)

* Surfacing better error messages about SQLCMD var names

* correcting docstring

* adding space to join char
This commit is contained in:
Benjin Dubishar
2023-03-29 14:23:03 -07:00
committed by GitHub
parent 2a9705c495
commit edc2c5e200
5 changed files with 37 additions and 38 deletions

View File

@@ -123,6 +123,7 @@ export function updatedToSdkStyleError(projectName: string) { return localize('u
export const enterNewName = localize('enterNewName', "Enter new name");
//#endregion
export const illegalSqlCmdChars = ['$', '@', '#', '"', '\'', '-'];
export const reservedProjectFolders = ['Properties', 'SQLCMD Variables', 'Database References'];
//#region Publish dialog strings
@@ -393,6 +394,7 @@ export const prePostDeployCount = localize('prePostDeployCount', "To successfull
export const invalidProjectReload = localize('invalidProjectReload', "Cannot access provided database project. Only valid, open database projects can be reloaded.");
export const externalStreamingJobValidationPassed = localize('externalStreamingJobValidationPassed', "Validation of external streaming job passed.");
export const errorRetrievingBuildFiles = localize('errorRetrievingBuildFiles', "Could not build project. Error retrieving files needed to build.");
export function projectAlreadyOpened(path: string) { return localize('projectAlreadyOpened', "Project '{0}' is already opened.", path); }
export function projectAlreadyExists(name: string, path: string) { return localize('projectAlreadyExists', "A project named {0} already exists in {1}.", name, path); }
export function noFileExist(fileName: string) { return localize('noFileExist', "File {0} doesn't exist", fileName); }
@@ -412,7 +414,6 @@ export function unexpectedProjectContext(uri: string) { return localize('unexpec
export function unableToPerformAction(action: string, uri: string) { return localize('unableToPerformAction', "Unable to locate '{0}' target: '{1}'", action, uri); }
export function unableToFindObject(path: string, objType: string) { return localize('unableToFindFile', "Unable to find {1} with path '{0}'", path, objType); }
export function deployScriptExists(scriptType: string) { return localize('deployScriptExists', "A {0} script already exists. The new script will not be included in build.", scriptType); }
export function notValidVariableName(name: string) { return localize('notValidVariableName', "The variable name '{0}' is not valid.", name); }
export function cantAddCircularProjectReference(project: string) { return localize('cantAddCircularProjectReference', "A reference to project '{0}' cannot be added. Adding this project as a reference would cause a circular dependency", project); }
export function unableToFindSqlCmdVariable(variableName: string) { return localize('unableToFindSqlCmdVariable', "Unable to find SQLCMD variable '{0}'", variableName); }
export function unableToFindDatabaseReference(reference: string) { return localize('unableToFindReference', "Unable to find database reference {0}", reference); }
@@ -421,6 +422,8 @@ export function invalidTargetPlatform(targetPlatform: string, supportedTargetPla
export function errorReadingProject(section: string, path: string, error?: string) { return localize('errorReadingProjectGuid', "Error trying to read {0} of project '{1}'. {2}", section, path, error); }
export function errorAddingDatabaseReference(referenceName: string, error: string) { return localize('errorAddingDatabaseReference', "Error adding database reference to {0}. Error: {1}", referenceName, error); }
export function errorNotSupportedInVsCode(actionDescription: string) { return localize('errorNotSupportedInVsCode', "Error: {0} is not currently supported in SQL Database Projects for VS Code.", actionDescription); }
export function sqlcmdVariableNameCannotContainWhitespace(name: string) { return localize('sqlcmdVariableNameCannotBeWhitespace', "SQLCMD variable name '{0}' cannot contain whitespace", name); }
export function sqlcmdVariableNameCannotContainIllegalChars(name: string) { return localize('sqlcmdVariableNameCannotContainIllegalChars', "SQLCMD variable name '{0}' cannot contain any of the following characters: {1}", name, illegalSqlCmdChars.join(', ')); }
//#endregion

View File

@@ -242,25 +242,24 @@ export function formatSqlCmdVariable(name: string): string {
* Checks if it's a valid sqlcmd variable name
* https://docs.microsoft.com/en-us/sql/ssms/scripting/sqlcmd-use-with-scripting-variables?redirectedfrom=MSDN&view=sql-server-ver15#guidelines-for-scripting-variable-names-and-values
* @param name variable name to validate
*/
export function isValidSqlCmdVariableName(name: string | undefined): boolean {
* @returns null if valid, otherwise an error message describing why input is invalid
*/
export function validateSqlCmdVariableName(name: string | undefined): string | null {
// remove $() around named if it's there
name = removeSqlCmdVariableFormatting(name);
const cleanedName = removeSqlCmdVariableFormatting(name);
// can't contain whitespace
if (!name || name.trim() === '' || name.includes(' ')) {
return false;
if (!cleanedName || cleanedName.trim() === '' || cleanedName.includes(' ')) {
return constants.sqlcmdVariableNameCannotContainWhitespace(name ?? '');
}
// can't contain these characters
if (name.includes('$') || name.includes('@') || name.includes('#') || name.includes('"') || name.includes('\'') || name.includes('-')) {
return false;
if (constants.illegalSqlCmdChars.some(c => cleanedName?.includes(c))) {
return constants.sqlcmdVariableNameCannotContainIllegalChars(name ?? '');
}
// TODO: tsql parsing to check if it's a reserved keyword or invalid tsql https://github.com/microsoft/azuredatastudio/issues/12204
// TODO: give more detail why variable name was invalid https://github.com/microsoft/azuredatastudio/issues/12231
return true;
return null;
}
/**

View File

@@ -564,9 +564,7 @@ export class AddDatabaseReferenceDialog {
// check for invalid variables
if (!this.validSqlCmdVariables()) {
let invalidName = !utils.isValidSqlCmdVariableName(this.databaseVariableTextbox?.value) ? this.databaseVariableTextbox!.value! : this.serverVariableTextbox!.value!;
invalidName = utils.removeSqlCmdVariableFormatting(invalidName);
newText = constants.notValidVariableName(invalidName);
newText = utils.validateSqlCmdVariableName(this.databaseVariableTextbox?.value) ?? utils.validateSqlCmdVariableName(this.serverVariableTextbox!.value!)!;
}
this.exampleUsage!.value = newText;
@@ -574,8 +572,8 @@ export class AddDatabaseReferenceDialog {
}
private validSqlCmdVariables(): boolean {
if (this.databaseVariableTextbox?.enabled && this.databaseVariableTextbox?.value && !utils.isValidSqlCmdVariableName(this.databaseVariableTextbox?.value)
|| this.serverVariableTextbox?.enabled && this.serverVariableTextbox?.value && !utils.isValidSqlCmdVariableName(this.serverVariableTextbox?.value)) {
if (this.databaseVariableTextbox?.enabled && this.databaseVariableTextbox?.value && utils.validateSqlCmdVariableName(this.databaseVariableTextbox?.value)
|| this.serverVariableTextbox?.enabled && this.serverVariableTextbox?.value && utils.validateSqlCmdVariableName(this.serverVariableTextbox?.value)) {
return false;
}

View File

@@ -6,7 +6,7 @@
import path = require('path');
import * as vscode from 'vscode';
import * as constants from '../common/constants';
import { getSqlProjectsInWorkspace, isValidSqlCmdVariableName } from '../common/utils';
import { getSqlProjectsInWorkspace, validateSqlCmdVariableName } from '../common/utils';
import { DbServerValues, populateResultWithVars } from './utils';
import { AddDatabaseReferenceSettings } from '../controllers/projectController';
import { IDacpacReferenceSettings, IProjectReferenceSettings, ISystemDatabaseReferenceSettings } from '../models/IDatabaseReferenceSettings';
@@ -211,7 +211,7 @@ async function promptDbVar(defaultValue: string): Promise<string> {
title: constants.databaseVariable,
value: defaultValue,
validateInput: (value: string) => {
return isValidSqlCmdVariableName(value) ? '' : constants.notValidVariableName(value);
return validateSqlCmdVariableName(value) ?? '';
},
ignoreFocusOut: true
}) ?? '';
@@ -235,7 +235,7 @@ async function promptServerVar(): Promise<string> {
title: constants.serverVariable,
value: constants.otherSeverVariable,
validateInput: (value: string) => {
return isValidSqlCmdVariableName(value) ? '' : constants.notValidVariableName(value);
return validateSqlCmdVariableName(value) ?? '';
},
ignoreFocusOut: true
}) ?? '';

View File

@@ -58,29 +58,28 @@ describe('Tests to verify utils functions', function (): void {
it('Should determine invalid sqlcmd variable names', () => {
// valid names
should(utils.isValidSqlCmdVariableName('$(test)')).equal(true);
should(utils.isValidSqlCmdVariableName('$(test )')).equal(true, 'trailing spaces should be valid because they will be trimmed');
should(utils.isValidSqlCmdVariableName('test')).equal(true);
should(utils.isValidSqlCmdVariableName('test ')).equal(true, 'trailing spaces should be valid because they will be trimmed');
should(utils.isValidSqlCmdVariableName('$(test')).equal(true);
should(utils.isValidSqlCmdVariableName('$(test ')).equal(true, 'trailing spaces should be valid because they will be trimmed');
should(utils.validateSqlCmdVariableName('$(test)')).equal(null);
should(utils.validateSqlCmdVariableName('$(test )')).equal(null, 'trailing spaces should be valid because they will be trimmed');
should(utils.validateSqlCmdVariableName('test')).equal(null);
should(utils.validateSqlCmdVariableName('test ')).equal(null, 'trailing spaces should be valid because they will be trimmed');
should(utils.validateSqlCmdVariableName('$(test')).equal(null);
should(utils.validateSqlCmdVariableName('$(test ')).equal(null, 'trailing spaces should be valid because they will be trimmed');
// whitespace
should(utils.isValidSqlCmdVariableName('')).equal(false);
should(utils.isValidSqlCmdVariableName(' ')).equal(false);
should(utils.isValidSqlCmdVariableName(' ')).equal(false);
should(utils.isValidSqlCmdVariableName('test abc')).equal(false);
should(utils.isValidSqlCmdVariableName(' ')).equal(false);
should(utils.validateSqlCmdVariableName('')).equal(constants.sqlcmdVariableNameCannotContainWhitespace(''));
should(utils.validateSqlCmdVariableName(' ')).equal(constants.sqlcmdVariableNameCannotContainWhitespace(' '));
should(utils.validateSqlCmdVariableName(' ')).equal(constants.sqlcmdVariableNameCannotContainWhitespace(' '));
should(utils.validateSqlCmdVariableName('test abc')).equal(constants.sqlcmdVariableNameCannotContainWhitespace('test abc'));
should(utils.validateSqlCmdVariableName(' ')).equal(constants.sqlcmdVariableNameCannotContainWhitespace(' '));
// invalid characters
should(utils.isValidSqlCmdVariableName('$($test')).equal(false);
should(utils.isValidSqlCmdVariableName('$test')).equal(false);
should(utils.isValidSqlCmdVariableName('$test')).equal(false);
should(utils.isValidSqlCmdVariableName('test@')).equal(false);
should(utils.isValidSqlCmdVariableName('test#')).equal(false);
should(utils.isValidSqlCmdVariableName('test"')).equal(false);
should(utils.isValidSqlCmdVariableName('test\'')).equal(false);
should(utils.isValidSqlCmdVariableName('test-1')).equal(false);
should(utils.validateSqlCmdVariableName('$($test')).equal(constants.sqlcmdVariableNameCannotContainIllegalChars('$($test'));
should(utils.validateSqlCmdVariableName('$test')).equal(constants.sqlcmdVariableNameCannotContainIllegalChars('$test'));
should(utils.validateSqlCmdVariableName('test@')).equal(constants.sqlcmdVariableNameCannotContainIllegalChars('test@'));
should(utils.validateSqlCmdVariableName('test#')).equal(constants.sqlcmdVariableNameCannotContainIllegalChars('test#'));
should(utils.validateSqlCmdVariableName('test"')).equal(constants.sqlcmdVariableNameCannotContainIllegalChars('test"'));
should(utils.validateSqlCmdVariableName('test\'')).equal(constants.sqlcmdVariableNameCannotContainIllegalChars('test\''));
should(utils.validateSqlCmdVariableName('test-1')).equal(constants.sqlcmdVariableNameCannotContainIllegalChars('test-1'));
});
it('Should convert from milliseconds to hr min sec correctly', () => {