Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Commit

Permalink
Added an InformativeDocsRule
Browse files Browse the repository at this point in the history
For any JSDoc comment on something with a name, if the comment only rephrases the name, this complains. Takes in configurable word aliases and a list of "useless" words that don't add to comments. I expect we'll slowly find more words to add to those defaults.

Fixes #457.
  • Loading branch information
Joshua Goldberg committed Oct 14, 2018
1 parent 7e87ab2 commit 5fd3055
Show file tree
Hide file tree
Showing 3 changed files with 453 additions and 0 deletions.
179 changes: 179 additions & 0 deletions src/informativeDocsRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
import * as Lint from 'tslint';
import * as ts from 'typescript';

import { getApparentJsDoc, getNodeName } from './utils/NodeDocs';

const defaultUselessWords = ['a', 'an', 'of', 'our', 'the'];

const defaultAliases: { [i: string]: string[] } = {
a: ['an', 'our'],
};

const failureString = 'This comment is roughly the same as the object\'s name. Either be more informative or don\'t include a comment.';

interface RawOptions {
aliases?: { [i: string]: string[] };
uselessWords?: string[];
}

interface Options {
aliases: Map<string, string>;
uselessWords: Set<string>;
}

export class Rule extends Lint.Rules.AbstractRule {
public static metadata: Lint.IRuleMetadata = {
description: 'Enforces that comments do more than just reiterate names of objects.',
options: null,
optionsDescription: 'Not configurable.',
optionExamples: [
true,
[true, {
aliases: {
a: ['an', 'our'],
emoji: ['smiley']
},
uselessWords: [...defaultUselessWords, 'also']
}]
],
rationale: Lint.Utils.dedent`
The documentation for an object should not be equivalent to just the object's name.
If we strip out non-alphabet characters, common words such as "the" or "a",
and lowercase everything, they shouldn't be the same.
Using informative documentation can be helpful for variables to help explain their usage.
Alternately, if something's name is so descriptive that it doesn't need to be fully documented,
just leave out documentation altogether.
`,
ruleName: 'informative-docs',
type: 'maintainability',
typescriptOnly: false
};

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithFunction(sourceFile, walk, parseOptions(this.getOptions().ruleArguments));
}
}

function parseOptions(ruleArguments: any[]): Options {
const rawOptions: RawOptions = ruleArguments.length === 0
? {}
: ruleArguments[0];

return {
aliases: parseAliasesOption(
rawOptions.aliases === undefined
? defaultAliases
: rawOptions.aliases
),
uselessWords: new Set(rawOptions.uselessWords === undefined ? defaultUselessWords : rawOptions.uselessWords)
};
}

function parseAliasesOption(rawAliases: { [i: string]: string[] }): Map<string, string> {
const aliases = new Map<string, string>();

for (const alias of Object.keys(rawAliases)) {
for (const aliasedName of rawAliases[alias]) {
aliases.set(aliasedName.toLowerCase(), alias.toLowerCase());
}
}

return aliases;
}

function walk(context: Lint.WalkContext<Options>) {
const { aliases, uselessWords } = context.options;

function nodeNameContainsUsefulWords(nameWords: string[], docWords: string[]): boolean {
const realDocWords = new Set(docWords);

for (const nameWord of nameWords) {
realDocWords.delete(nameWord);
}

uselessWords.forEach((uselessWord: string): void => {
realDocWords.delete(uselessWord);
});

return realDocWords.size !== 0;
}

function normalizeWord(word: string): string {
word = word.toLowerCase();

const aliasedWord = aliases.get(word);
if (aliasedWord !== undefined) {
word = aliasedWord;
}

return word;
}

function splitNameIntoWords(name: string): string[] | undefined {
if (name.length > 2 && name[0] === 'I' && Lint.Utils.isUpperCase(name[1])) {
name = name.substring(1);
}

const nameSpaced = name
.replace(/\W/g, '')
.replace(/([a-z])([A-Z])/g, '$1 $2')
.trim();

if (nameSpaced.length === 0) {
return undefined;
}

return nameSpaced
.split(' ')
.map(normalizeWord);
}

function getNodeDocComments(node: ts.Node): string[] | undefined {
const docsRaw = getApparentJsDoc(node);
if (docsRaw === undefined) {
return undefined;
}

const docs = docsRaw
.map((doc) => doc.comment)
.filter((comment) => comment !== undefined);

if (docs.length === 0) {
return undefined;
}

return docs
.join(' ')
.replace(/[^A-Za-z0-9 ]/g, '')
.split(' ')
.map(normalizeWord);
}

function verifyNodeWithName(node: ts.Node, name: string): void {
const docs = getNodeDocComments(node);
if (docs === undefined) {
return;
}

const nameSplit = splitNameIntoWords(name);
if (nameSplit === undefined) {
return;
}

if (!nodeNameContainsUsefulWords(nameSplit, docs)) {
context.addFailureAtNode(node, failureString);
}
}

function visitNode(node: ts.Node): void {
const name = getNodeName(node);
if (name !== undefined) {
verifyNodeWithName(node, name);
}

return ts.forEachChild(node, visitNode);
}

return ts.forEachChild(context.sourceFile, visitNode);
}
192 changes: 192 additions & 0 deletions src/tests/InformativeDocsRuleTests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
import { TestHelper } from './TestHelper';

describe('informativeDocsRule', () : void => {
it('should pass on well-described functions', () : void => {
const script : string = `
/**
* Does X Y Z work.
*/
function foo() {}
/**
* Transforms the processed data.
*/
function bar() {}
/**
* Transforms the processed baz data.
*/
function bazProcessor() {}
`;

TestHelper.assertViolations('informative-docs', script, []);
});

it('should pass on a non-commented function', () : void => {
const script : string = `
function foo() {}
`;

TestHelper.assertViolations('informative-docs', script, []);
});

it('should fail on uninformative docs comments of all kinds', () : void => {
const script : string = `
/**
* Foo class
*/
class FooClass {
/**
* the foo method declaration
*/
fooMethodDeclaration() {}
/**
* The foo property declaration.
*/
public fooPropertyDeclaration;
}
/**
* the Foo enum.
*/
enum FooEnum {}
/**
* The foo function
*/
function fooFunction() {}
const _ = {
/**
* The foo get accessor.
*/
get fooGetAccessor() {
return 0;
}
};
/**
* The foo interface.
*/
interface IFooInterface {}
/**
* The foo module.
*/
module FooModule {}
/**
* The foo type.
*/
type fooType = {};
`;

TestHelper.assertViolations('informative-docs', script, [
{
failure: 'This comment is roughly the same as the object\'s name. Either be more informative or don\'t include a comment.',
name: 'file.ts',
ruleName: 'informative-docs',
startPosition: { character: 13, line: 5 }
},
{
failure: 'This comment is roughly the same as the object\'s name. Either be more informative or don\'t include a comment.',
name: 'file.ts',
ruleName: 'informative-docs',
startPosition: { character: 17, line: 9 }
},
{
failure: 'This comment is roughly the same as the object\'s name. Either be more informative or don\'t include a comment.',
name: 'file.ts',
ruleName: 'informative-docs',
startPosition: { character: 17, line: 14 }
},
{
failure: 'This comment is roughly the same as the object\'s name. Either be more informative or don\'t include a comment.',
name: 'file.ts',
ruleName: 'informative-docs',
startPosition: { character: 13, line: 20 }
},
{
failure: 'This comment is roughly the same as the object\'s name. Either be more informative or don\'t include a comment.',
name: 'file.ts',
ruleName: 'informative-docs',
startPosition: { character: 13, line: 25 }
},
{
failure: 'This comment is roughly the same as the object\'s name. Either be more informative or don\'t include a comment.',
name: 'file.ts',
ruleName: 'informative-docs',
startPosition: { character: 17, line: 31 }
},
{
failure: 'This comment is roughly the same as the object\'s name. Either be more informative or don\'t include a comment.',
name: 'file.ts',
ruleName: 'informative-docs',
startPosition: { character: 13, line: 39 }
},
{
failure: 'This comment is roughly the same as the object\'s name. Either be more informative or don\'t include a comment.',
name: 'file.ts',
ruleName: 'informative-docs',
startPosition: { character: 13, line: 44 }
},
{
failure: 'This comment is roughly the same as the object\'s name. Either be more informative or don\'t include a comment.',
name: 'file.ts',
ruleName: 'informative-docs',
startPosition: { character: 13, line: 49 }
}
]);
});

it('should fail on uninformative docs comments using alias options', () : void => {
const script : string = `
/**
* The FancyFoo.
*/
function foo() {}
`;

const options = [
{
aliases: {
foo: ['FancyFoo']
}
}
];

TestHelper.assertViolationsWithOptions('informative-docs', options, script, [
{
failure: 'This comment is roughly the same as the object\'s name. Either be more informative or don\'t include a comment.',
name: 'file.ts',
ruleName: 'informative-docs',
startPosition: { character: 13, line: 5 }
}
]);
});

it('should fail on uninformative docs comments using useless word options', () : void => {
const script : string = `
/**
* Also foo.
*/
function foo() {}
`;

const options = [
{
uselessWords: ['also']
}
];

TestHelper.assertViolationsWithOptions('informative-docs', options, script, [
{
failure: 'This comment is roughly the same as the object\'s name. Either be more informative or don\'t include a comment.',
name: 'file.ts',
ruleName: 'informative-docs',
startPosition: { character: 13, line: 5 }
}
]);
});
});
Loading

0 comments on commit 5fd3055

Please sign in to comment.