-
-
Notifications
You must be signed in to change notification settings - Fork 42
Change Request: Support getParent, getAncestors, getText and lines as a optional values for SourceCodeBase interface #175
Description
Which packages would you like to change?
-
@eslint/compat -
@eslint/config-array -
@eslint/config-helpers -
@eslint/core -
@eslint/migrate-config -
@eslint/object-schema -
@eslint/plugin-kit
What problem do you want to solve?
Hello,
While reviewing the newly updated type definition for the IMarkdownSourceCode interface implemented in @eslint/markdown, I discovered critical bugs stemming from inconsistencies with the SourceCodeBase interface and the TextSourceCodeBase class.
Where is this bug originated from?
Currently, TextSourceCodeBase implements SourceCodeBase, meaning that the TextSourceCodeBase class must implement all methods and properties defined in SourceCodeBase.
This is not a problem as long as the SourceCodeBase class only includes methods and properties that exist in the TextSourceCodeBase interface—since they are in sync. That is, all methods and properties in SourceCodeBase exist in TextSourceCodeBase and vice versa.
However, issues arise when the SourceCodeBase class overextends by implementing additional methods and properties that are not part of the TextSourceCodeBase interface.
If you examine the source code of SourceCodeBase class, you'll notice that getParent, getAncestors, getText, and lines are not included in the TextSourceCodeBase interface.
While SourceCodeBase class is allowed to define additional methods and properties beyond TextSourceCodeBase interface, problems emerge when a class like MarkdownSoureCode(from @eslint/markdown) extends TextSourceCodeBase, and IMarkdownSourceCode interface extends TextSourceCode interface,
This raises an important question: where do getParent, getAncestors, getText and lines go?
Since IMarkdownSourceCode does not inherit directly from SourceCodeBase class, it does not include these methods. As a result, IMarkdownSourceCode cannot reuse the existing implementations, forcing us to redefine these types manually.
This leads to errors when trying to access getParent, getAncestors, getText, or lines from context, as seen in the following error message:
What do you think is the correct solution?
I can think of three possible solutions to this issue.
First
Import MarkdownSourceCode from markdown-source-code.js and use it instead of IMarkdownSourceCode interface in the type definitions.
This approach is already used in the JSON.
+ import {
+ MarkdownSourceCode
+ } from "./language/markdown-source-code.js";
// ...
export type MarkdownRuleDefinition<
Options extends Partial<MarkdownRuleDefinitionTypeOptions> = {},
> = RuleDefinition<
// Language specific type options (non-configurable)
{
LangOptions: {};
- Code: IMarkdownSourceCode;
+ Code: MarkdownSourceCode;
Visitor: MarkdownRuleVisitor;
Node: Node;
} & Required<
// Rule specific type options (custom)
Options &
// Rule specific type options (defaults)
Omit<MarkdownRuleDefinitionTypeOptions, keyof Options>
>
>;Second
Define getParent, getAncestors, getText and lines as optional properties in the SourceCodeBase interface. This way, they remain part of the type definition but without requiring every implementation to include them.
interface SourceCodeBase<
Options extends SourceCodeBaseTypeOptions = {
LangOptions: LanguageOptions;
RootNode: unknown;
SyntaxElementWithLoc: unknown;
ConfigNode: unknown;
},
> {
// ...
/**
* Returns an array of all inline configuration nodes found in the
* source code.
*/
getInlineConfigNodes?(): Options["ConfigNode"][];
/**
* Applies configuration found inside of the source code. This method is only
* called when ESLint is running with inline configuration allowed.
*/
applyInlineConfig?(): {
configs: InlineConfigElement[];
problems: FileProblem[];
};
// ...
getParent?(): {
// implementation
}
getAncestors?(): {
// implementation
}
getText?(): {
// implementation
}
lines?: // implementation
}Third
Create a new type definition specifically for TextSourceCodeBase that includes getParent, getAncestors, getText, and lines. This would allow IMarkdownSourceCode to extend this new type instead of TextSourceCodeBase, ensuring type consistency.
Participation
- I am willing to submit a pull request for this change.
Additional comments
The first way would be the easiest way to fix this problem. However, I can't say for sure if it's the approach the ESLint team intends. So, I ended up writing a long issue.
This issue is closely related to the issues and PRs such as eslint/markdown#333, eslint/eslint#19521, eslint/markdown#324.
Looking forward to hearing the ESLint team's thoughts!
Metadata
Metadata
Assignees
Labels
Type
Projects
Status
