Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Observer nullability fixes #5349

Merged
3 changes: 1 addition & 2 deletions src/observers/BackgroundWorkStatusBarObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@ export class BackgroundWorkStatusBarObserver extends BaseStatusBarItemObserver {

if (asProjectEvent.message.Status === DiagnosticStatus.Processing) {
let projectFile = asProjectEvent.message.ProjectFilePath.replace(/^.*[\\\/]/, '');
this.SetAndShowStatusBar(`$(sync~spin) Analyzing ${projectFile}`, 'o.showOutput', null, `Analyzing ${projectFile}`);
this.SetAndShowStatusBar(`$(sync~spin) Analyzing ${projectFile}`, 'o.showOutput', undefined, `Analyzing ${projectFile}`);
}
else {
this.ResetAndHideStatusBar();
}
}
}
}

4 changes: 2 additions & 2 deletions src/observers/BaseStatusBarItemObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ export abstract class BaseStatusBarItemObserver {
}

public ResetAndHideStatusBar() {
this.statusBarItem.text = undefined;
this.statusBarItem.text = '';
this.statusBarItem.command = undefined;
this.statusBarItem.color = undefined;
this.statusBarItem.tooltip = undefined;
this.statusBarItem.hide();
}

abstract post: (event: BaseEvent) => void;
}
}
4 changes: 2 additions & 2 deletions src/observers/CsharpLoggerObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { PackageError } from "../packageManager/PackageError";
import { EventType } from "../omnisharp/EventType";

export class CsharpLoggerObserver extends BaseLoggerObserver {
private dots: number;
private dots: number = 0;

public post = (event: Event.BaseEvent) => {
switch (event.type) {
Expand Down Expand Up @@ -146,4 +146,4 @@ export class CsharpLoggerObserver extends BaseLoggerObserver {
private handleDocumentSynchronizationFailure(event: Event.DocumentSynchronizationFailure) {
this.logger.appendLine(`Failed to synchronize document '${event.documentPath}': ${event.errorMessage}`);
}
}
}
2 changes: 1 addition & 1 deletion src/observers/OmnisharpLoggerObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export class OmnisharpLoggerObserver extends BaseLoggerObserver {
this.logger.append(`OmniSharp server started`);
if (event.hostVersion) {
this.logger.append(` with ${event.hostIsMono ? 'Mono' : '.NET'} ${event.hostVersion}`);
if (event.hostPath?.length > 0) {
if (event.hostPath && event.hostPath.length > 0) {
this.logger.append(` (${event.hostPath})`);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/observers/OptionProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { Options } from "../omnisharp/options";
import { Subscription, Observable } from "rxjs";
export default class OptionProvider {
private options: Options;
private options?: Options;
private subscription: Subscription;

constructor(optionObservable: Observable<Options>) {
Expand All @@ -24,4 +24,4 @@ export default class OptionProvider {
public dispose = () => {
this.subscription.unsubscribe();
}
}
}
12 changes: 6 additions & 6 deletions src/observers/TelemetryObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ export interface ITelemetryReporter {
export class TelemetryObserver {
private reporter: ITelemetryReporter;
private platformInfo: PlatformInformation;
private solutionId: string;
private dotnetInfo: DotnetInfo;
private solutionId?: string;
private dotnetInfo?: DotnetInfo;

constructor(platformInfo: PlatformInformation, reporterCreator: () => ITelemetryReporter) {
this.platformInfo = platformInfo;
Expand Down Expand Up @@ -64,7 +64,7 @@ export class TelemetryObserver {
}

private handleTelemetryEventMeasures(event: TelemetryEventWithMeasures) {
this.reporter.sendTelemetryEvent(event.eventName, null, event.measures);
this.reporter.sendTelemetryEvent(event.eventName, undefined, event.measures);
}

private async handleOmnisharpInitialisation(event: OmnisharpInitialisation) {
Expand All @@ -90,16 +90,16 @@ export class TelemetryObserver {

private handleTestExecutionCountReport(event: TestExecutionCountReport) {
if (event.debugCounts) {
this.reporter.sendTelemetryEvent('DebugTest', null, event.debugCounts);
this.reporter.sendTelemetryEvent('DebugTest', undefined, event.debugCounts);
}
if (event.runCounts) {
this.reporter.sendTelemetryEvent('RunTest', null, event.runCounts);
this.reporter.sendTelemetryEvent('RunTest', undefined, event.runCounts);
}
}

private handleProjectConfigurationReceived(event: ProjectConfiguration, telemetryProps: { [key: string]: string }) {
let projectConfig = event.projectConfiguration;
telemetryProps['SolutionId'] = this.solutionId;
telemetryProps['SolutionId'] = this.solutionId ?? "";
telemetryProps['ProjectId'] = projectConfig.ProjectId;
telemetryProps['SessionId'] = projectConfig.SessionId;
telemetryProps['OutputType'] = projectConfig.OutputKind?.toString() ?? "";
Expand Down
6 changes: 3 additions & 3 deletions src/observers/utils/ShowInformationMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import MessageItemWithCommand from "./MessageItemWithCommand";

export default async function showInformationMessage(vscode: vscode, message: string, ...items: MessageItemWithCommand[]) {
try {
let value = await vscode.window.showInformationMessage<MessageItemWithCommand>(message, ...items);
if (value && value.command) {
const value = await vscode.window.showInformationMessage<MessageItemWithCommand>(message, ...items);
if (value?.command) {
vscode.commands.executeCommand(value.command);
}
}
catch (err) {
console.log(err);
}
}
}
6 changes: 3 additions & 3 deletions src/observers/utils/ShowWarningMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import MessageItemWithCommand from "./MessageItemWithCommand";

export default async function showWarningMessage(vscode: vscode, message: string, ...items: MessageItemWithCommand[]) {
try {
let value = await vscode.window.showWarningMessage<MessageItemWithCommand>(message, ...items);
if (value && value.command) {
const value = await vscode.window.showWarningMessage<MessageItemWithCommand>(message, ...items);
if (value?.command) {
await vscode.commands.executeCommand<string>(value.command);
}
}
catch (err) {
console.log(err);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@ suite('BackgroundWorkStatusBarObserver', () => {
observer.post(event);
expect(hideCalled).to.be.true;
expect(showCalled).to.be.false;
expect(statusBarItem.text).to.be.undefined;
expect(statusBarItem.text).to.be.equal('');
});
});
});
8 changes: 4 additions & 4 deletions test/unitTests/logging/OmnisharpStatusBarObserver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ suite('OmnisharpStatusBarObserver', () => {
let hideCalled: boolean;

setup(() => {
statusBarItem.text = undefined;
statusBarItem.text = '';
statusBarItem.color = undefined;
statusBarItem.command = undefined;
statusBarItem.tooltip = undefined;
Expand Down Expand Up @@ -84,7 +84,7 @@ suite('OmnisharpStatusBarObserver', () => {
let event = new OmnisharpServerOnStop();
observer.post(event);
expect(hideCalled).to.be.true;
expect(statusBarItem.text).to.be.undefined;
expect(statusBarItem.text).to.be.equal('');
expect(statusBarItem.command).to.be.undefined;
expect(statusBarItem.color).to.be.undefined;
});
Expand Down Expand Up @@ -118,8 +118,8 @@ suite('OmnisharpStatusBarObserver', () => {
observer.post(successEvent);

expect(hideCalled).to.be.true;
expect(statusBarItem.text).to.be.undefined;
expect(statusBarItem.text).to.be.equal('');
expect(statusBarItem.command).to.be.undefined;
expect(statusBarItem.color).to.be.undefined;
});
});
});
10 changes: 5 additions & 5 deletions test/unitTests/logging/ProjectStatusBarObserver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ suite('ProjectStatusBarObserver', () => {
let event = new OmnisharpServerOnStop();
observer.post(event);
expect(hideCalled).to.be.true;
expect(statusBarItem.text).to.be.undefined;
expect(statusBarItem.text).to.be.equal('');
expect(statusBarItem.command).to.be.undefined;
expect(statusBarItem.color).to.be.undefined;
});
Expand All @@ -44,19 +44,19 @@ suite('ProjectStatusBarObserver', () => {

suite('WorkspaceInformationUpdated', () => {
test('Project status is hidden if there is no MSBuild Object', () => {
let event = getWorkspaceInformationUpdated(null);
let event = getWorkspaceInformationUpdated(undefined);
observer.post(event);
expect(hideCalled).to.be.true;
expect(statusBarItem.text).to.be.undefined;
expect(statusBarItem.text).to.be.equal('');
expect(statusBarItem.command).to.be.undefined;
});

test('Project status is shown if there is an MSBuild object', () => {
let event = getWorkspaceInformationUpdated(getMSBuildWorkspaceInformation("somePath", []));
observer.post(event);
expect(showCalled).to.be.true;
expect(statusBarItem.text).to.contain(event.info.MsBuild.SolutionPath);
expect(statusBarItem.text).to.contain(event.info.MsBuild?.SolutionPath);
expect(statusBarItem.command).to.equal('o.pickProjectAndStart');
});
});
});
});
12 changes: 6 additions & 6 deletions test/unitTests/options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,33 +40,33 @@ suite("Options tests", () => {

test('Verify return no excluded paths when files.exclude empty', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, null, 'files.exclude', {});
updateConfig(vscode, undefined, 'files.exclude', {});

const excludedPaths = Options.getExcludedPaths(vscode);
expect(excludedPaths).to.be.empty;
});

test('Verify return excluded paths when files.exclude populated', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, null, 'files.exclude', { "**/node_modules": true, "**/assets": false });
updateConfig(vscode, undefined, 'files.exclude', { "**/node_modules": true, "**/assets": false });

const excludedPaths = Options.getExcludedPaths(vscode);
expect(excludedPaths).to.equalTo(["**/node_modules"]);
});

test('Verify return no excluded paths when files.exclude and search.exclude empty', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, null, 'files.exclude', {});
updateConfig(vscode, null, 'search.exclude', {});
updateConfig(vscode, undefined, 'files.exclude', {});
updateConfig(vscode, undefined, 'search.exclude', {});

const excludedPaths = Options.getExcludedPaths(vscode, true);
expect(excludedPaths).to.be.empty;
});

test('Verify return excluded paths when files.exclude and search.exclude populated', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, null, 'files.exclude', { "/Library": true });
updateConfig(vscode, null, 'search.exclude', { "**/node_modules": true, "**/assets": false });
updateConfig(vscode, undefined, 'files.exclude', { "/Library": true });
updateConfig(vscode, undefined, 'search.exclude', { "**/node_modules": true, "**/assets": false });

const excludedPaths = Options.getExcludedPaths(vscode, true);
expect(excludedPaths).to.be.equalTo(["/Library", "**/node_modules"]);
Expand Down
47 changes: 21 additions & 26 deletions test/unitTests/testAssets/Fakes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ export const getWorkspaceConfiguration = (): vscode.WorkspaceConfiguration => {
let configuration: vscode.WorkspaceConfiguration = {
get<T>(section: string, defaultValue?: T): T | undefined {
let result = <T>values[section];
return result === undefined && defaultValue !== undefined
? defaultValue
: result;
return result ?? defaultValue;
},
has: (section: string) => {
return values[section] !== undefined;
Expand Down Expand Up @@ -148,9 +146,9 @@ export function getFakeVsCode(): vscode.vscode {
},
version: "",
env: {
appName: null,
appRoot: null,
language: null,
appName: "",
appRoot: "",
language: "",
clipboard: {
writeText: () => {
throw new Error("Not Implemented");
Expand All @@ -159,8 +157,8 @@ export function getFakeVsCode(): vscode.vscode {
throw new Error("Not Implemented");
}
},
machineId: null,
sessionId: null,
machineId: "",
sessionId: "",
openExternal: () => {
throw new Error("Not Implemented");
}
Expand All @@ -175,12 +173,10 @@ export function getMSBuildWorkspaceInformation(msBuildSolutionPath: string, msBu
};
}

export function getWorkspaceInformationUpdated(msbuild: protocol.MsBuildWorkspaceInformation): WorkspaceInformationUpdated {
let a: protocol.WorkspaceInformationResponse = {
MsBuild: msbuild
};

return new WorkspaceInformationUpdated(a);
export function getWorkspaceInformationUpdated(MsBuild: protocol.MsBuildWorkspaceInformation | undefined): WorkspaceInformationUpdated {
return new WorkspaceInformationUpdated({
MsBuild,
});
}

export function getVSCodeWithConfig() {
Expand All @@ -189,27 +185,26 @@ export function getVSCodeWithConfig() {
const _vscodeConfig = getWorkspaceConfiguration();
const _omnisharpConfig = getWorkspaceConfiguration();
const _csharpConfig = getWorkspaceConfiguration();
const _razorConfig = getWorkspaceConfiguration();

vscode.workspace.getConfiguration = (section?, resource?) => {
if (!section) {
vscode.workspace.getConfiguration = (section, resource) => {
if (section === undefined) {
return _vscodeConfig;
}

if (section === 'omnisharp') {
} else if (section === 'omnisharp') {
return _omnisharpConfig;
}

if (section === 'csharp') {
} else if (section === 'csharp') {
return _csharpConfig;
} else if (section === 'razor') {
return _razorConfig;
}

return undefined;
throw new Error(`Unexpected section ${section}`);
};

return vscode;
}

export function updateConfig(vscode: vscode.vscode, section: string, config: string, value: any) {
let workspaceConfig = vscode.workspace.getConfiguration(section);
export function updateConfig(vscode: vscode.vscode, section: string | undefined, config: string, value: any) {
const workspaceConfig = vscode.workspace.getConfiguration(section);
workspaceConfig.update(config, value);
}
}