Skip to content

FIX: @W-16371174@: Fix bug with config commands case insensitivity #1705

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.

Sign up for GitHub

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

Merged
stephen-carter-at-sf merged 2 commits into from
Dec 27, 2024
Merged

FIX: @W-16371174@: Fix bug with config commands case insensitivity #1705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16ca560
FIX: @W-16371174@: Fix bug with config commands case insensitivity
stephen-carter-at-sf Dec 27, 2024
48922f2
Update release state message to Beta
stephen-carter-at-sf Dec 27, 2024
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
2 changes: 1 addition & 1 deletion messages/shared.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# label.command-state

Developer Preview
Beta
Copy link
Contributor Author

@stephen-carter-at-sf stephen-carter-at-sf Dec 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[FIX-AS-YOU-GO item] We don't want to forget to update this to say "Beta" instead of "Developer Preview" now.


# warning.command-state

Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"bugs": "https://github.com/forcedotcom/sfdx-scanner/issues",
"dependencies": {
"@oclif/core": "^3.3.2",
"@salesforce/code-analyzer-core": "0.20.1",
"@salesforce/code-analyzer-core": "0.20.2",
Copy link
Contributor Author

@stephen-carter-at-sf stephen-carter-at-sf Dec 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main fix

"@salesforce/code-analyzer-engine-api": "0.16.1",
"@salesforce/code-analyzer-eslint-engine": "0.17.0",
"@salesforce/code-analyzer-flowtest-engine": "0.16.2",
Expand Down
4 changes: 2 additions & 2 deletions src/commands/code-analyzer/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default class ConfigCommand extends SfCommand<void> implements Displayabl
public static readonly description = getMessage(BundleName.ConfigCommand, 'command.description');
public static readonly examples = getMessages(BundleName.ConfigCommand, 'command.examples');

// TODO: Update when we go to Beta and when we go GA
// TODO: Remove when we go GA
public static readonly state = getMessage(BundleName.Shared, 'label.command-state');

public static readonly flags = {
Expand Down Expand Up @@ -50,7 +50,7 @@ export default class ConfigCommand extends SfCommand<void> implements Displayabl
};

public async run(): Promise<void> {
// TODO: Update when we go to Beta and when we go GA
// TODO: Remove when we go GA
this.warn(getMessage(BundleName.Shared, "warning.command-state", [getMessage(BundleName.Shared, 'label.command-state')]));

const parsedFlags = (await this.parse(ConfigCommand)).flags;
Expand Down
4 changes: 2 additions & 2 deletions src/commands/code-analyzer/rules.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default class RulesCommand extends SfCommand<void> implements Displayable
public static readonly description = getMessage(BundleName.RulesCommand, 'command.description');
public static readonly examples = getMessages(BundleName.RulesCommand, 'command.examples');

// TODO: Update when we go to Beta and when we go GA
// TODO: Remove when we go GA
public static readonly state = getMessage(BundleName.Shared, 'label.command-state');

public static readonly flags = {
Expand Down Expand Up @@ -52,7 +52,7 @@ export default class RulesCommand extends SfCommand<void> implements Displayable
};

public async run(): Promise<void> {
// TODO: Update when we go to Beta and when we go GA
// TODO: Remove when we go GA
this.warn(getMessage(BundleName.Shared, "warning.command-state", [getMessage(BundleName.Shared, 'label.command-state')]));

const parsedFlags = (await this.parse(RulesCommand)).flags;
Expand Down
4 changes: 2 additions & 2 deletions src/commands/code-analyzer/run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export default class RunCommand extends SfCommand<void> implements Displayable {
public static readonly description = getMessage(BundleName.RunCommand, 'command.description');
public static readonly examples = getMessages(BundleName.RunCommand, 'command.examples');

// TODO: Update when we go to Beta and when we go GA
// TODO: Remove when we go GA
public static readonly state = getMessage(BundleName.Shared, 'label.command-state');

public static readonly flags = {
Expand Down Expand Up @@ -78,7 +78,7 @@ export default class RunCommand extends SfCommand<void> implements Displayable {
};

public async run(): Promise<void> {
// TODO: Update when we go to Beta and when we go GA
// TODO: Remove when we go GA
this.warn(getMessage(BundleName.Shared, "warning.command-state", [getMessage(BundleName.Shared, 'label.command-state')]));

const parsedFlags = (await this.parse(RunCommand)).flags;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/actions/ConfigAction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export class ConfigAction {
// We need the Set of all Engines that returned rules for the user's selection on both the Default and User Cores.
const relevantEngines: Set<string> = new Set([...userRules.getEngineNames(), ...selectedDefaultRules.getEngineNames()]);

const configModel: ConfigModel = new AnnotatedConfigModel(userConfig, userCore, userRules, allDefaultRules, relevantEngines);
const configModel: ConfigModel = new AnnotatedConfigModel(userCore, userRules, allDefaultRules, relevantEngines);

const fileWritten: boolean = this.dependencies.writer
? await this.dependencies.writer.write(configModel)
Expand Down
20 changes: 9 additions & 11 deletions src/lib/models/ConfigModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ export interface ConfigModel {
}

export class AnnotatedConfigModel implements ConfigModel {
private readonly config: CodeAnalyzerConfig; // TODO: It would be nice if we updated the CodeAnalyzer (in our core module) to just return its CodeAnalyzerConfig with a getter so we didn't need to pass it around
Copy link
Contributor Author

@stephen-carter-at-sf stephen-carter-at-sf Dec 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the other enhancements that I just pulled in, I was able to accomplish this TODO item as well.

private readonly codeAnalyzer: CodeAnalyzer;
private readonly userRules: RuleSelection;
private readonly allDefaultRules: RuleSelection;
Expand All @@ -34,8 +33,7 @@ export class AnnotatedConfigModel implements ConfigModel {
// configs not associated with the user's rule selection, thus we can't use the engines from allDefaultRules.
private readonly relevantEngines: Set<string>;

constructor(config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
this.config = config;
constructor(codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
this.codeAnalyzer = codeAnalyzer;
this.userRules = userRules;
this.allDefaultRules = allDefaultRules;
Expand All @@ -45,9 +43,9 @@ export class AnnotatedConfigModel implements ConfigModel {
toFormattedOutput(format: OutputFormat): string {
// istanbul ignore else: Should be impossible
if (format === OutputFormat.STYLED_YAML) {
return new StyledYamlFormatter(this.config, this.codeAnalyzer, this.userRules, this.allDefaultRules, this.relevantEngines).toYaml();
return new StyledYamlFormatter(this.codeAnalyzer, this.userRules, this.allDefaultRules, this.relevantEngines).toYaml();
} else if (format === OutputFormat.RAW_YAML) {
return new PlainYamlFormatter(this.config, this.codeAnalyzer, this.userRules, this.allDefaultRules, this.relevantEngines).toYaml();
return new PlainYamlFormatter(this.codeAnalyzer, this.userRules, this.allDefaultRules, this.relevantEngines).toYaml();
} else {
throw new Error(`Unsupported`)
}
Expand All @@ -61,8 +59,8 @@ abstract class YamlFormatter {
private readonly allDefaultRules: RuleSelection;
private readonly relevantEngines: Set<string>;

protected constructor(config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
this.config = config;
protected constructor(codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
this.config = codeAnalyzer.getConfig();
this.codeAnalyzer = codeAnalyzer;
this.userRules = userRules;
this.allDefaultRules = allDefaultRules;
Expand Down Expand Up @@ -214,8 +212,8 @@ abstract class YamlFormatter {
}

class PlainYamlFormatter extends YamlFormatter {
constructor(config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
super(config, codeAnalyzer, userRules, allDefaultRules, relevantEngines);
constructor(codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
super(codeAnalyzer, userRules, allDefaultRules, relevantEngines);
}

protected toYamlComment(commentText: string): string {
Expand All @@ -224,8 +222,8 @@ class PlainYamlFormatter extends YamlFormatter {
}

class StyledYamlFormatter extends YamlFormatter {
constructor(config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
super(config, codeAnalyzer, userRules, allDefaultRules, relevantEngines);
constructor(codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
super(codeAnalyzer, userRules, allDefaultRules, relevantEngines);
}

protected toYamlComment(commentText: string): string {
Expand Down
9 changes: 2 additions & 7 deletions src/lib/viewers/ResultsViewer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,11 @@ export class ResultsTableDisplayer extends AbstractResultsDisplayer {
protected _view(results: RunResults) {
const violations: Violation[] = sortViolations(results.getViolations());
const parentFolder: string = findLongestCommonParentFolderOf(violations.map(v =>
getPrimaryLocation(v).getFile()).filter(f => f !== undefined));
v.getPrimaryLocation().getFile()).filter(f => f !== undefined));

const resultRows: ResultRow[] = violations.map((v, idx) => {
const severity = v.getRule().getSeverityLevel();
const primaryLocation = getPrimaryLocation(v);
const primaryLocation: CodeLocation = v.getPrimaryLocation();
const relativeFile: string | null = primaryLocation.getFile() ?
path.relative(parentFolder, primaryLocation.getFile()!) : null;
return {
Expand Down Expand Up @@ -193,11 +193,6 @@ function sortViolations(violations: Violation[]): Violation[] {
});
}

// TODO: We should update core module to have this function directly on the Violation object and then remove this helper
Copy link
Contributor Author

@stephen-carter-at-sf stephen-carter-at-sf Dec 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the other enhancements that I just pulled in, I was able to accomplish this TODO item as well.

function getPrimaryLocation(violation: Violation): CodeLocation {
return violation.getCodeLocations()[violation.getPrimaryLocationIndex()];
}

/**
* Returns the longest comment parent folder of the file paths provided
* Note that this function assumes that all the paths are indeed files and not folders
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1699,10 +1699,10 @@
strip-ansi "6.0.1"
ts-retry-promise "^0.8.1"

"@salesforce/code-analyzer-core@0.20.1":
version "0.20.1"
resolved "https://registry.yarnpkg.com/@salesforce/code-analyzer-core/-/code-analyzer-core-0.20.1.tgz#319fa83aa645cec6d14d8b0ad97cbfef505b1cee"
integrity sha512-c/xqGoB2Naea5+xbE/N8dcIC5gt1Fety1QRs2CKrXgv23y2YzYi7W6u7hmGZ9SlqtT0RvT3OngdvY7qgiaLV0Q==
"@salesforce/code-analyzer-core@0.20.2":
version "0.20.2"
resolved "https://registry.yarnpkg.com/@salesforce/code-analyzer-core/-/code-analyzer-core-0.20.2.tgz#bc33c27809929070473e9dd94ef0fcf77f4a23c6"
integrity sha512-b1PqZD2lAW7XUcDjYAS+V40CpHQsLysdfq7Elo9ap6eCasjQ15CTOsctJWjyv2ZpUTyphAYo9iSDjkLZCJD2eg==
dependencies:
"@salesforce/code-analyzer-engine-api" "0.16.1"
"@types/js-yaml" "^4.0.9"
Expand Down
Loading