Skip to content
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

Add 'Symbol.toStringTag' into every publicly exported class #3170

Merged
merged 1 commit into from Jun 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .eslintrc.yml
Expand Up @@ -22,6 +22,7 @@ rules:

internal-rules/only-ascii: error
internal-rules/no-dir-import: error
internal-rules/require-to-string-tag: off

##############################################################################
# `eslint-plugin-istanbul` rule list based on `v0.1.2`
Expand Down Expand Up @@ -610,8 +611,12 @@ overrides:
'@typescript-eslint/space-before-function-paren': off
'@typescript-eslint/space-infix-ops': off
'@typescript-eslint/type-annotation-spacing': off
- files: 'src/**'
rules:
internal-rules/require-to-string-tag: error
- files: 'src/**/__*__/**'
rules:
internal-rules/require-to-string-tag: off
node/no-unpublished-import: [error, { allowModules: ['chai', 'mocha'] }]
import/no-restricted-paths: off
import/no-extraneous-dependencies: [error, { devDependencies: true }]
Expand Down
2 changes: 2 additions & 0 deletions resources/eslint-internal-rules/index.js
Expand Up @@ -2,10 +2,12 @@

const onlyASCII = require('./only-ascii.js');
const noDirImport = require('./no-dir-import.js');
const requireToStringTag = require('./require-to-string-tag.js');

module.exports = {
rules: {
'only-ascii': onlyASCII,
'no-dir-import': noDirImport,
'require-to-string-tag': requireToStringTag,
},
};
37 changes: 37 additions & 0 deletions resources/eslint-internal-rules/require-to-string-tag.js
@@ -0,0 +1,37 @@
'use strict';

module.exports = function requireToStringTag(context) {
const sourceCode = context.getSourceCode();

return {
'ExportNamedDeclaration > ClassDeclaration': (classNode) => {
const properties = classNode.body.body;
if (properties.some(isToStringTagProperty)) {
return;
}

const jsDoc = context.getJSDocComment(classNode)?.value;
// FIXME: use proper TSDoc parser instead of includes once we fix TSDoc comments
if (jsDoc?.includes('@internal') === true) {
return;
}

context.report({
node: classNode,
message:
'All classes in public API required to have [Symbol.toStringTag] method',
});
},
};

function isToStringTagProperty(propertyNode) {
if (
propertyNode.type !== 'MethodDefinition' ||
propertyNode.kind !== 'get'
) {
return false;
}
const keyText = sourceCode.getText(propertyNode.key);
return keyText === 'Symbol.toStringTag';
}
};
9 changes: 7 additions & 2 deletions src/language/__tests__/lexer-test.ts
Expand Up @@ -114,8 +114,13 @@ describe('Lexer', () => {
});
});

it('can be JSON.stringified, util.inspected or jsutils.inspect', () => {
const token = lexOne('foo');
it('can be Object.toStringified, JSON.stringified, or jsutils.inspected', () => {
const lexer = new Lexer(new Source('foo'));
const token = lexer.advance();

expect(Object.prototype.toString.call(lexer)).to.equal('[object Lexer]');

expect(Object.prototype.toString.call(token)).to.equal('[object Token]');
expect(JSON.stringify(token)).to.equal(
'{"kind":"Name","value":"foo","line":1,"column":1}',
);
Expand Down
9 changes: 5 additions & 4 deletions src/language/__tests__/parser-test.ts
Expand Up @@ -372,11 +372,12 @@ describe('Parser', () => {
expect(() => parse(document)).to.throw('Syntax Error');
});

it('contains location information that only stringifies start/end', () => {
const result = parse('{ id }');
it('contains location that can be Object.toStringified, JSON.stringified, or jsutils.inspected', () => {
const { loc } = parse('{ id }');

expect(JSON.stringify(result.loc)).to.equal('{"start":0,"end":6}');
expect(inspect(result.loc)).to.equal('{ start: 0, end: 6 }');
expect(Object.prototype.toString.call(loc)).to.equal('[object Location]');
expect(JSON.stringify(loc)).to.equal('{"start":0,"end":6}');
expect(inspect(loc)).to.equal('{ start: 0, end: 6 }');
});

it('contains references to source', () => {
Expand Down
8 changes: 8 additions & 0 deletions src/language/ast.ts
Expand Up @@ -42,6 +42,10 @@ export class Location {
toJSON(): { start: number; end: number } {
return { start: this.start, end: this.end };
}

get [Symbol.toStringTag]() {
return 'Location';
}
}

/**
Expand Down Expand Up @@ -121,6 +125,10 @@ export class Token {
column: this.column,
};
}

get [Symbol.toStringTag]() {
return 'Token';
}
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/language/lexer.ts
Expand Up @@ -79,6 +79,10 @@ export class Lexer {
}
return token;
}

get [Symbol.toStringTag]() {
return 'Lexer';
}
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/utilities/TypeInfo.ts
Expand Up @@ -292,6 +292,10 @@ export class TypeInfo {
break;
}
}

get [Symbol.toStringTag]() {
return 'TypeInfo';
}
}

type GetFieldDefFn = (
Expand Down
8 changes: 8 additions & 0 deletions src/utilities/__tests__/TypeInfo-test.ts
Expand Up @@ -15,6 +15,14 @@ import { TypeInfo, visitWithTypeInfo } from '../TypeInfo';
import { testSchema } from '../../validation/__tests__/harness';

describe('TypeInfo', () => {
it('can be Object.toStringified', () => {
const typeInfo = new TypeInfo(testSchema);

expect(Object.prototype.toString.call(typeInfo)).to.equal(
'[object TypeInfo]',
);
});

it('allow all methods to be called before entering any node', () => {
const typeInfo = new TypeInfo(testSchema);

Expand Down
12 changes: 12 additions & 0 deletions src/validation/ValidationContext.ts
Expand Up @@ -131,6 +131,10 @@ export class ASTValidationContext {
}
return fragments;
}

get [Symbol.toStringTag]() {
return 'ASTValidationContext';
}
}

export type ASTValidationRule = (context: ASTValidationContext) => ASTVisitor;
Expand All @@ -150,6 +154,10 @@ export class SDLValidationContext extends ASTValidationContext {
getSchema(): Maybe<GraphQLSchema> {
return this._schema;
}

get [Symbol.toStringTag]() {
return 'SDLValidationContext';
}
}

export type SDLValidationRule = (context: SDLValidationContext) => ASTVisitor;
Expand Down Expand Up @@ -253,6 +261,10 @@ export class ValidationContext extends ASTValidationContext {
getEnumValue(): Maybe<GraphQLEnumValue> {
return this._typeInfo.getEnumValue();
}

get [Symbol.toStringTag]() {
return 'ValidationContext';
}
}

export type ValidationRule = (context: ValidationContext) => ASTVisitor;
40 changes: 40 additions & 0 deletions src/validation/__tests__/ValidationContext-test.ts
@@ -0,0 +1,40 @@
import { expect } from 'chai';
import { describe, it } from 'mocha';

import { identityFunc } from '../../jsutils/identityFunc';

import { parse } from '../../language/parser';

import { GraphQLSchema } from '../../type/schema';

import { TypeInfo } from '../../utilities/TypeInfo';

import {
ASTValidationContext,
SDLValidationContext,
ValidationContext,
} from '../ValidationContext';

describe('ValidationContext', () => {
it('can be Object.toStringified', () => {
const schema = new GraphQLSchema({});
const typeInfo = new TypeInfo(schema);
const ast = parse('{ foo }');
const onError = identityFunc;

const astContext = new ASTValidationContext(ast, onError);
expect(Object.prototype.toString.call(astContext)).to.equal(
'[object ASTValidationContext]',
);

const sdlContext = new SDLValidationContext(ast, schema, onError);
expect(Object.prototype.toString.call(sdlContext)).to.equal(
'[object SDLValidationContext]',
);

const context = new ValidationContext(schema, ast, typeInfo, onError);
expect(Object.prototype.toString.call(context)).to.equal(
'[object ValidationContext]',
);
});
});