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

feat: improve parsing #49

Merged
merged 2 commits into from
Apr 10, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions src/NodeNatspecParser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { Natspec, NatspecDefinition, NodeToProcess } from './types';

export class NodeNatspecParser {
/**
* Parses the natspec of the node
* @param {NodeToProcess} node - The node to process
* @returns {Natspec} - The parsed natspec
*/
parse(node: NodeToProcess): Natspec {
if (!node.documentation) {
return { tags: [], params: [], returns: [] };
}

let currentTag: NatspecDefinition | null = null;
const result: Natspec = {
tags: [],
params: [],
returns: [],
};

const docText: string = typeof node.documentation === 'string' ? node.documentation : node.documentation.text;

docText.split('\n').forEach((line) => {
const tagTypeMatch = line.match(/^\s*@(\w+)/);
if (tagTypeMatch) {
const tagName = tagTypeMatch[1];

if (tagName === 'inheritdoc') {
const tagMatch = line.match(/^\s*@(\w+) (.*)$/);
if (tagMatch) {
currentTag = null;
result.inheritdoc = { content: tagMatch[2] };
}
} else if (tagName === 'param' || tagName === 'return') {
const tagMatch = line.match(/^\s*@(\w+) *(\w*) *(.*)$/);
if (tagMatch) {
currentTag = { name: tagMatch[2], content: tagMatch[3].trim() };
result[tagName === 'param' ? 'params' : 'returns'].push(currentTag);
}
} else {
const tagMatch = line.match(/^\s*@(\w+) *(.*)$/);
if (tagMatch) {
currentTag = { name: tagName, content: tagMatch[2] };
result.tags.push(currentTag);
}
}
} else if (currentTag) {
currentTag.content += '\n' + line;
}
});

return result;
}
}
2 changes: 1 addition & 1 deletion src/config/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import { Config } from '../types';
* @returns {Config} - The config
*/
export function getConfig(configPath: string): Config {
const fileConfig = getFileConfig(configPath);
const argConfig = getArgsConfig();
const fileConfig = getFileConfig(configPath);
// Merge default config with file config and arg config
const inputConfig = _.merge(_.merge(_.cloneDeep(defaultConfig), fileConfig), argConfig);

Expand Down
4 changes: 3 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { getProjectCompiledSources } from './utils';
import { Processor } from './processor';
import { Config } from './types';
import { Validator } from './validator';
import { NodeNatspecParser } from './NodeNatspecParser';
import { getConfig } from './config';

/**
Expand All @@ -23,7 +24,8 @@ import { getConfig } from './config';
if (!sourceUnits.length) return console.error('No solidity files found in the specified directory');

const validator = new Validator(config);
const processor = new Processor(validator);
const parser = new NodeNatspecParser();
const processor = new Processor(validator, parser);
const warnings = await processor.processSources(sourceUnits);

if (!warnings.length) {
Expand Down
8 changes: 5 additions & 3 deletions src/processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ import fs from 'fs/promises';
import { SourceUnit, FunctionDefinition, ContractDefinition } from 'solc-typed-ast';
import { Validator } from './validator';
import { NodeToProcess } from './types';
import { getLineNumberFromSrc, parseNodeNatspec } from './utils';
import { getLineNumberFromSrc } from './utils';
import { NodeNatspecParser } from './NodeNatspecParser';

export interface IWarning {
location: string;
messages: string[];
}

export class Processor {
constructor(private validator: Validator) {}
constructor(private validator: Validator, private nodeNatspecParser: NodeNatspecParser) {}

/**
* Goes through all functions, modifiers, state variables, structs, enums, errors and events
Expand Down Expand Up @@ -70,7 +72,7 @@ export class Processor {
* @returns {string[]} - The list of warning messages
*/
validateNatspec(node: NodeToProcess): string[] {
const nodeNatspec = parseNodeNatspec(node);
const nodeNatspec = this.nodeNatspecParser.parse(node);
return this.validator.validate(node, nodeNatspec);
}

Expand Down
48 changes: 24 additions & 24 deletions test/parser.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { ContractDefinition } from 'solc-typed-ast';
import { parseNodeNatspec } from '../src/utils';
import { getFileCompiledSource, findNode } from './utils/helpers';
import { mockNatspec } from './utils/mocks';
import { NodeNatspecParser } from '../src/NodeNatspecParser';

describe('Parser', () => {
describe('NodeNatspecParser', () => {
let contract: ContractDefinition;

const nodeNatspecParser = new NodeNatspecParser();
describe('Contract', () => {
beforeAll(async () => {
const compileResult = await getFileCompiledSource('test/contracts/ParserTest.sol');
Expand All @@ -14,7 +14,7 @@ describe('Parser', () => {

it('should parse the inheritdoc tag', async () => {
const node = findNode(contract.vFunctions, 'viewFunctionNoParams');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand All @@ -31,7 +31,7 @@ describe('Parser', () => {

it('should parse constant', async () => {
const node = findNode(contract.vStateVariables, 'SOME_CONSTANT');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand All @@ -44,7 +44,7 @@ describe('Parser', () => {

it('should parse variable', async () => {
const node = findNode(contract.vStateVariables, 'someVariable');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand All @@ -57,7 +57,7 @@ describe('Parser', () => {

it('should parse modifier', async () => {
const node = findNode(contract.vModifiers, 'someModifier');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand All @@ -79,7 +79,7 @@ describe('Parser', () => {

it('should parse external function', async () => {
const node = findNode(contract.vFunctions, 'viewFunctionNoParams');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand All @@ -98,7 +98,7 @@ describe('Parser', () => {

it('should parse private function', async () => {
const node = findNode(contract.vFunctions, '_viewPrivate');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand Down Expand Up @@ -130,7 +130,7 @@ describe('Parser', () => {

it('should parse multiline descriptions', async () => {
const node = findNode(contract.vFunctions, '_viewMultiline');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand All @@ -146,7 +146,7 @@ describe('Parser', () => {

it('should parse multiple of the same tag', async () => {
const node = findNode(contract.vFunctions, '_viewDuplicateTag');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand All @@ -173,7 +173,7 @@ describe('Parser', () => {

it('should parse error', async () => {
const node = findNode(contract.vErrors, 'SimpleError');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand All @@ -189,7 +189,7 @@ describe('Parser', () => {

it('should parse event', async () => {
const node = findNode(contract.vEvents, 'SimpleEvent');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand All @@ -205,7 +205,7 @@ describe('Parser', () => {

it('should parse struct', async () => {
const node = findNode(contract.vStructs, 'SimplestStruct');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand Down Expand Up @@ -233,7 +233,7 @@ describe('Parser', () => {

it('should parse external function without parameters', async () => {
const node = findNode(contract.vFunctions, 'viewFunctionNoParams');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand All @@ -259,7 +259,7 @@ describe('Parser', () => {

it('should parse external function with parameters', async () => {
const node = findNode(contract.vFunctions, 'viewFunctionWithParams');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand Down Expand Up @@ -298,7 +298,7 @@ describe('Parser', () => {

it('should parse struct', async () => {
const node = findNode(contract.vStructs, 'SimpleStruct');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand All @@ -309,7 +309,7 @@ describe('Parser', () => {

it('should parse inheritdoc + natspec', async () => {
const node = findNode(contract.vStateVariables, 'someVariable');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand All @@ -328,21 +328,21 @@ describe('Parser', () => {

it('should not parse the inheritdoc tag with just 2 slashes', async () => {
const node = findNode(contract.vStateVariables, 'SOME_CONSTANT');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(mockNatspec({}));
});

it('should not parse regular comments as natspec', async () => {
const node = findNode(contract.vFunctions, 'viewFunctionWithParams');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(mockNatspec({}));
});

it('should parse natspec with multiple spaces', async () => {
const node = findNode(contract.vFunctions, '_viewPrivate');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand Down Expand Up @@ -370,14 +370,14 @@ describe('Parser', () => {

it('should not parse natspec with invalid number of slashes', async () => {
const node = findNode(contract.vFunctions, '_viewInternal');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(mockNatspec({}));
});

it('should parse natspec with invalid formatting', async () => {
const node = findNode(contract.vFunctions, '_viewLinterFail');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand All @@ -397,7 +397,7 @@ describe('Parser', () => {

it('should correctly parse empty return tag', async () => {
const node = findNode(contract.vFunctions, 'functionUnnamedEmptyReturn');
const result = parseNodeNatspec(node);
const result = nodeNatspecParser.parse(node);

expect(result).toEqual(
mockNatspec({
Expand Down
6 changes: 4 additions & 2 deletions test/processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import * as utils from '../src/utils';
import { Validator } from '../src/validator';
import { mockFunctionDefinition, mockNodeToProcess, mockConfig, mockNatspec } from './utils/mocks';
import { getFileCompiledSource } from './utils/helpers';
import { NodeNatspecParser } from '../src/NodeNatspecParser';

describe('Processor', () => {
const validator = new Validator(mockConfig({}));
const processor = new Processor(validator);
const parser = new NodeNatspecParser();
const processor = new Processor(validator, parser);

describe('selectEligibleNodes', () => {
let contract: ContractDefinition;
Expand Down Expand Up @@ -96,7 +98,7 @@ describe('Processor', () => {
const nodeNatspec = mockNatspec({});

it('should parse the natspec of the node', () => {
const parseNodeNatspecSpy = jest.spyOn(utils, 'parseNodeNatspec').mockImplementation((_) => nodeNatspec);
const parseNodeNatspecSpy = jest.spyOn(parser, 'parse').mockImplementation((_) => nodeNatspec);

processor.validateNatspec(node);

Expand Down
10 changes: 10 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,16 @@ ajv@^8.11.0:
require-from-string "^2.0.2"
uri-js "^4.2.2"

ajv@^6.12.4:
version "6.12.6"
resolved "https://registry.yarnpkg.com/ajv/-/ajv-6.12.6.tgz#baf5a62e802b07d977034586f8c3baf5adf26df4"
integrity sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==
dependencies:
fast-deep-equal "^3.1.1"
fast-json-stable-stringify "^2.0.0"
json-schema-traverse "^0.4.1"
uri-js "^4.2.2"

ansi-escapes@^4.2.1, ansi-escapes@^4.3.0:
version "4.3.2"
resolved "https://registry.npmjs.org/ansi-escapes/-/ansi-escapes-4.3.2.tgz"
Expand Down
Loading