Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"changes": [
{
"packageName": "@rushstack/eslint-plugin",
"comment": "Fix no-new-null false positives for ECMAScript private class members.",
"type": "patch"
}
]
}
9 changes: 7 additions & 2 deletions eslint/eslint-plugin/src/no-new-null.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ type Options = [];

interface IAccessible {
accessibility?: TSESTree.Accessibility;
key?: TSESTree.Node;
}

const noNewNullRule: TSESLint.RuleModule<MessageIds, Options> = {
Expand Down Expand Up @@ -37,11 +38,15 @@ const noNewNullRule: TSESLint.RuleModule<MessageIds, Options> = {

create: (context: TSESLint.RuleContext<MessageIds, Options>) => {
/**
* Returns true if the accessibility is not explicitly set to private or protected, e.g. class properties, methods.
* Returns true unless a class member uses protected, TypeScript-private, or ECMAScript-private syntax.
*/
function isPubliclyAccessible(node?: IAccessible): boolean {
const accessibility: TSESTree.Accessibility | undefined = node?.accessibility;
return !(accessibility === 'private' || accessibility === 'protected');
return (
accessibility !== 'private' &&
accessibility !== 'protected' &&
node?.key?.type !== AST_NODE_TYPES.PrivateIdentifier
);
}

/**
Expand Down
11 changes: 11 additions & 0 deletions eslint/eslint-plugin/src/test/no-new-null.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,17 @@ ruleTester.run('no-new-null', noNewNullRule, {
' }',
'}'
].join('\n')
},
{
code: [
'class NativePrivateNulls {',
' #field: string | null;',
' #propertyFunc: (value: string | null) => void;',
' #method(value: string | null): string | null { return value; }',
' get #value(): string | null { return this.#field; }',
' set #value(value: string | null) { this.#field = value; }',
'}'
].join('\n')
}
]
});
48 changes: 24 additions & 24 deletions libraries/rush-lib/src/api/ApprovedPackagesConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,39 +59,39 @@ const _jsonSchema: JsonSchema = JsonSchema.fromLoadedObject(schemaJson);
export class ApprovedPackagesConfiguration {
public items: ApprovedPackagesItem[] = [];

private _itemsByName: Map<string, ApprovedPackagesItem> = new Map<string, ApprovedPackagesItem>();
#itemsByName: Map<string, ApprovedPackagesItem> = new Map<string, ApprovedPackagesItem>();

private _loadedJson!: IApprovedPackagesJson;
private _jsonFilename: string;
#loadedJson!: IApprovedPackagesJson;
#jsonFilename: string;

public constructor(jsonFilename: string) {
this._jsonFilename = jsonFilename;
this.#jsonFilename = jsonFilename;
this.clear();
}

/**
* Clears all the settings, returning to an empty state.
*/
public clear(): void {
this._itemsByName.clear();
this._loadedJson = {
this.#itemsByName.clear();
this.#loadedJson = {
// Ensure this comes first in the key ordering
$schema: '',
packages: []
};
}

public getItemByName(packageName: string): ApprovedPackagesItem | undefined {
return this._itemsByName.get(packageName);
return this.#itemsByName.get(packageName);
}

public addOrUpdatePackage(packageName: string, reviewCategory: string): boolean {
let changed: boolean = false;

let item: ApprovedPackagesItem | undefined = this._itemsByName.get(packageName);
let item: ApprovedPackagesItem | undefined = this.#itemsByName.get(packageName);
if (!item) {
item = new ApprovedPackagesItem(packageName);
this._addItem(item);
this.#addItem(item);
changed = true;
}

Expand All @@ -107,7 +107,7 @@ export class ApprovedPackagesConfiguration {
* If the file exists, calls loadFromFile().
*/
public tryLoadFromFile(approvedPackagesPolicyEnabled: boolean): boolean {
if (!FileSystem.exists(this._jsonFilename)) {
if (!FileSystem.exists(this.#jsonFilename)) {
return false;
}

Expand All @@ -116,7 +116,7 @@ export class ApprovedPackagesConfiguration {
if (!approvedPackagesPolicyEnabled) {
// eslint-disable-next-line no-console
console.log(
`Warning: Ignoring "${path.basename(this._jsonFilename)}" because the` +
`Warning: Ignoring "${path.basename(this.#jsonFilename)}" because the` +
` "approvedPackagesPolicy" setting was not specified in ${RushConstants.rushJsonFilename}`
);
}
Expand All @@ -129,14 +129,14 @@ export class ApprovedPackagesConfiguration {
*/
public loadFromFile(): void {
const approvedPackagesJson: IApprovedPackagesJson = JsonFile.loadAndValidate(
this._jsonFilename,
this.#jsonFilename,
_jsonSchema
);

this.clear();

for (const browserPackage of approvedPackagesJson.packages) {
this._addItemJson(browserPackage, this._jsonFilename);
this.#addItemJson(browserPackage, this.#jsonFilename);
}
}

Expand All @@ -148,9 +148,9 @@ export class ApprovedPackagesConfiguration {
// (which passed schema validation).

// eslint-disable-next-line dot-notation
this._loadedJson['$schema'] = JsonSchemaUrls.approvedPackages;
this.#loadedJson['$schema'] = JsonSchemaUrls.approvedPackages;

this._loadedJson.packages = [];
this.#loadedJson.packages = [];

this.items.sort((a: ApprovedPackagesItem, b: ApprovedPackagesItem) => {
return a.packageName.localeCompare(b.packageName);
Expand All @@ -166,11 +166,11 @@ export class ApprovedPackagesConfiguration {
allowedCategories: allowedCategories
};

this._loadedJson.packages.push(itemJson);
this.#loadedJson.packages.push(itemJson);
}

// Save the file
let body: string = JsonFile.stringify(this._loadedJson);
let body: string = JsonFile.stringify(this.#loadedJson);

// Unindent the allowedCategories array to improve readability
body = body.replace(/("allowedCategories": +\[)([^\]]+)/g, (substring: string, ...args: string[]) => {
Expand All @@ -180,16 +180,16 @@ export class ApprovedPackagesConfiguration {
// Add a header
body = '// DO NOT ADD COMMENTS IN THIS FILE. They will be lost when the Rush tool resaves it.\n' + body;

FileSystem.writeFile(this._jsonFilename, body, {
FileSystem.writeFile(this.#jsonFilename, body, {
convertLineEndings: NewlineKind.CrLf
});
}

/**
* Helper function only used by the constructor when loading the file.
*/
private _addItemJson(itemJson: IApprovedPackagesItemJson, jsonFilename: string): void {
if (this._itemsByName.has(itemJson.name)) {
#addItemJson(itemJson: IApprovedPackagesItemJson, jsonFilename: string): void {
if (this.#itemsByName.has(itemJson.name)) {
throw new Error(
`Error loading package review file ${jsonFilename}:\n` +
` the name "${itemJson.name}" appears more than once`
Expand All @@ -202,18 +202,18 @@ export class ApprovedPackagesConfiguration {
item.allowedCategories.add(allowedCategory);
}
}
this._addItem(item);
this.#addItem(item);
}

/**
* Helper function that adds an already created ApprovedPackagesItem to the
* list and set.
*/
private _addItem(item: ApprovedPackagesItem): void {
if (this._itemsByName.has(item.packageName)) {
#addItem(item: ApprovedPackagesItem): void {
if (this.#itemsByName.has(item.packageName)) {
throw new InternalError('Duplicate key');
}
this.items.push(item);
this._itemsByName.set(item.packageName, item);
this.#itemsByName.set(item.packageName, item);
}
}
28 changes: 14 additions & 14 deletions libraries/rush-lib/src/api/ChangeFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import { Git } from '../logic/Git';
* This class represents a single change file.
*/
export class ChangeFile {
private _changeFileData: IChangeFile;
private _rushConfiguration: RushConfiguration;
#changeFileData: IChangeFile;
#rushConfiguration: RushConfiguration;

/**
* @internal
Expand All @@ -30,16 +30,16 @@ export class ChangeFile {
throw new Error(`rushConfiguration does not have a value`);
}

this._changeFileData = changeFileData;
this._rushConfiguration = rushConfiguration;
this.#changeFileData = changeFileData;
this.#rushConfiguration = rushConfiguration;
}

/**
* Adds a change entry into the change file
* @param data - change information
*/
public addChange(data: IChangeInfo): void {
this._changeFileData.changes.push(data);
this.#changeFileData.changes.push(data);
}

/**
Expand All @@ -48,7 +48,7 @@ export class ChangeFile {
*/
public getChanges(packageName: string): IChangeInfo[] {
const changes: IChangeInfo[] = [];
for (const info of this._changeFileData.changes) {
for (const info of this.#changeFileData.changes) {
if (info.packageName === packageName) {
changes.push(info);
}
Expand All @@ -63,7 +63,7 @@ export class ChangeFile {
*/
public writeSync(): string {
const filePath: string = this.generatePath();
JsonFile.save(this._changeFileData, filePath, {
JsonFile.save(this.#changeFileData, filePath, {
ensureFolderExists: true
});
return filePath;
Expand All @@ -76,7 +76,7 @@ export class ChangeFile {
*/
public generatePath(): string {
let branch: string | undefined = undefined;
const git: Git = new Git(this._rushConfiguration);
const git: Git = new Git(this.#rushConfiguration);
const repoInfo: gitInfo.GitRepoInfo | undefined = git.getGitInfo();
branch = repoInfo && repoInfo.branch;
if (!branch) {
Expand All @@ -89,13 +89,13 @@ export class ChangeFile {
// flag rarely had any effect, and a second invocation would silently clobber the
// change file written by the first one. See GitHub issue #2195.
// example filename: yourbranchname_2017-05-01-20-20-30.json
const timestamp: string | undefined = this._getTimestamp(true);
const timestamp: string | undefined = this.#getTimestamp(true);
const filename: string = branch
? this._escapeFilename(`${branch}_${timestamp}.json`)
? this.#escapeFilename(`${branch}_${timestamp}.json`)
: `${timestamp}.json`;
const filePath: string = path.join(
this._rushConfiguration.changesFolder,
...this._changeFileData.packageName.split('/'),
this.#rushConfiguration.changesFolder,
...this.#changeFileData.packageName.split('/'),
filename
);
return filePath;
Expand All @@ -105,7 +105,7 @@ export class ChangeFile {
* Gets the current time, formatted as YYYY-MM-DD-HH-MM
* When useSeconds is true, the seconds are appended as well: YYYY-MM-DD-HH-MM-SS
*/
private _getTimestamp(useSeconds: boolean = false): string | undefined {
#getTimestamp(useSeconds: boolean = false): string | undefined {
// Create a date string with the current time

// dateString === "2016-10-19T22:47:49.606Z"
Expand Down Expand Up @@ -137,7 +137,7 @@ export class ChangeFile {
return undefined;
}

private _escapeFilename(filename: string, replacer: string = '-'): string {
#escapeFilename(filename: string, replacer: string = '-'): string {
// Removes / ? < > \ : * | ", really anything that isn't a letter, number, '.' '_' or '-'
const badCharacters: RegExp = /[^a-zA-Z0-9._-]/g;
return filename.replace(badCharacters, replacer);
Expand Down
24 changes: 12 additions & 12 deletions libraries/rush-lib/src/api/CobuildConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ export class CobuildConfiguration {
*/
public readonly cobuildWithoutCacheAllowed: boolean;

private _cobuildLockProvider: ICobuildLockProvider | undefined;
private readonly _cobuildLockProviderFactory: CobuildLockProviderFactory;
private readonly _cobuildJson: ICobuildJson;
#cobuildLockProvider: ICobuildLockProvider | undefined;
readonly #cobuildLockProviderFactory: CobuildLockProviderFactory;
readonly #cobuildJson: ICobuildJson;

private constructor(options: ICobuildConfigurationOptions) {
const { cobuildJson, cobuildLockProviderFactory, rushConfiguration } = options;
Expand All @@ -91,8 +91,8 @@ export class CobuildConfiguration {
this.cobuildWithoutCacheAllowed =
rushConfiguration.experimentsConfiguration.configuration.allowCobuildWithoutCache ?? false;

this._cobuildLockProviderFactory = cobuildLockProviderFactory;
this._cobuildJson = cobuildJson;
this.#cobuildLockProviderFactory = cobuildLockProviderFactory;
this.#cobuildJson = cobuildJson;
}

/**
Expand Down Expand Up @@ -127,25 +127,25 @@ export class CobuildConfiguration {
public async createLockProviderAsync(terminal: ITerminal): Promise<void> {
if (this.cobuildFeatureEnabled) {
terminal.writeLine(`Running cobuild (runner ${this.cobuildContextId}/${this.cobuildRunnerId})`);
const cobuildLockProvider: ICobuildLockProvider = await this._cobuildLockProviderFactory(
this._cobuildJson
const cobuildLockProvider: ICobuildLockProvider = await this.#cobuildLockProviderFactory(
this.#cobuildJson
);
this._cobuildLockProvider = cobuildLockProvider;
await this._cobuildLockProvider.connectAsync();
this.#cobuildLockProvider = cobuildLockProvider;
await this.#cobuildLockProvider.connectAsync();
}
}

public async destroyLockProviderAsync(): Promise<void> {
if (this.cobuildFeatureEnabled) {
await this._cobuildLockProvider?.disconnectAsync();
await this.#cobuildLockProvider?.disconnectAsync();
}
}

public getCobuildLockProvider(): ICobuildLockProvider {
if (!this._cobuildLockProvider) {
if (!this.#cobuildLockProvider) {
throw new Error(`Cobuild lock provider has not been created`);
}
return this._cobuildLockProvider;
return this.#cobuildLockProvider;
}
}

Expand Down
Loading
Loading