Skip to content

Change Request: Support getParent, getAncestors, getText and lines as a optional values for SourceCodeBase interface #175

@lumirlumir

Description

@lumirlumir

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:

Image

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

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    Status

    Complete

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions