Skip to content

NEW (eslint): @W-16113413@ Add configuration abilities to eslint engine #40

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

Merged
merged 6 commits into from
Jul 10, 2024

Conversation

stephen-carter-at-sf
Copy link
Collaborator

This PR contains:

  • Moved configuration validation utilities into the engine-api to be shared by the engines and core
  • Moved message catalog utility into engine-api to be shared by the engines and core
  • Added in eslint configuration definitions
  • Applied configuration to eslint engine and added in tests

@stephen-carter-at-sf stephen-carter-at-sf merged commit 8b5141c into dev Jul 10, 2024
5 checks passed
@stephen-carter-at-sf stephen-carter-at-sf deleted the sc/W-16113413 branch July 10, 2024 14:00
@stephen-carter-at-sf stephen-carter-at-sf restored the sc/W-16113413 branch July 10, 2024 14:08
});

it('When strings are provided which are not absolute paths that share a common root folder, then return null', () => {
expect(calculateLongestCommonParentFolderOf(['oops', 'nope'])).toStrictEqual(null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment explaining why we chose to return null, instead of an empty string or throwing an error?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See the comment on the calcualteLongestCommonParentFolderOf ... but i'll update it even more in the next PR with:

/**
 * Returns the longest common parent folder of the provided paths.
 *
 * @param paths It is assumed that paths is a non-empty array of absolute value paths.
 * If empty or if no common parent folder exists (like in the case on Windows machines of using two different drives
 * C: and D:) then null is returned (to allow the client of this function to handle this case without needing try/catch)
 */

testInvalidLineOrColumnScenario(1, 3.124, 3, 4, 'startColumn', 3.124);
testInvalidLineOrColumnScenario(1, 2, -1.2, 4, 'endLine', -1.2);
testInvalidLineOrColumnScenario(1, 2, 3, 0, 'endColumn', 0);
it("When an engine returns a code location that has an invalid line or column, then an error is thrown", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you rewrite this test to use it.each()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure.

getMessage('ConfigPathValueDoesNotExist','engines.dummy.some_field2', path.resolve(process.cwd(), 'doesNotExist')));
});

it("When calling extractFolder on a field that is a folder, then error", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

"field that is a file"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. Will fix in next PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants