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: forbid instance and static class members with same name #988

Merged
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
37 changes: 16 additions & 21 deletions docs/language/common/comments.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ with no special meaning.

Documentation comments support [Markdown](https://www.markdownguide.org/) to format the text. Here is an example:

```sds
```sds hl_lines="2"
/**
* This is a documentation comment with **bold** and *italic* text.
*/
Expand All @@ -86,40 +86,35 @@ Documentation comments can contain tags to provide structured information.
`{@link}` is an **inline** tag that can be used to link to another declaration. It takes the name of the declaration as
an argument:

```sds
```sds hl_lines="2"
/**
* Computes the sum of two {@link Int}s.
*/
fun sum(a: Int, b: Int): sum: Int
```

To point to _static_ members of a [class][class] or an [enum variant][enum-variant] of an [enum][enum], write the name of
the containing declaration followed by a dot and the name of the member or enum variant:
To point to a member of a [class][class] or an [enum variant][enum-variant] of an [enum][enum], write the name of the
containing declaration followed by a dot and the name of the member or enum variant:

```sds
```sds hl_lines="2"
/**
* To create a Table, use {@link Table.fromCsv}.
* To create a Configuration, use {@link Configuration.fromFile}.
*/
class Table
```
class Configuration {

To point to an _instance_ member of a [class][class], write the name of the containing declaration followed by a hash and
the name of the member:

```sds
/**
* An alias for {@link List#size}.
*/
fun size(list: List<Any?>): size: Int
/**
* Creates a Configuration from a file.
*/
fun fromFile(file: String) -> result: Configuration
}
```


#### `@param`

Use `@param` to document a [parameter][parameter] of a callable declaration. This tag takes the name of the parameter
and its description as arguments. Since a callable can have multiple parameters, this tag can be used multiple times.

```sds
```sds hl_lines="4 5"
/**
* ...
*
Expand All @@ -134,7 +129,7 @@ fun sum(a: Int, b: Int): sum: Int
Use `@result` to document a [result][result] of a callable declaration. This tag takes the name of the result and its
description as arguments. Since a callable can have multiple results, this tag can be used multiple times.

```sds
```sds hl_lines="4"
/**
* ...
*
Expand All @@ -149,7 +144,7 @@ Use `@typeParam` to document a [type parameter][type-parameter] of a generic dec
type parameter and its description as arguments. Since a generic declaration can have multiple type parameters, this
tag can be used multiple times.

```sds
```sds hl_lines="4"
/**
* ...
*
Expand All @@ -163,7 +158,7 @@ class List<T>
The `@since` tag can be used to specify when a declaration was added. It takes the version as argument and should be
used only once.

```sds
```sds hl_lines="4"
/**
* ...
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
} from '../generated/ast.js';
import { isEmpty } from '../../helpers/collections.js';
import { SafeDsServices } from '../safe-ds-module.js';
import { getClassMembers, getEnumVariants, isStatic } from '../helpers/nodeProperties.js';
import { getClassMembers, getEnumVariants } from '../helpers/nodeProperties.js';

const PARAM_TAG = 'param';
const RESULT_TAG = 'result';
Expand Down Expand Up @@ -186,49 +186,22 @@ export class SafeDsDocumentationProvider extends JSDocDocumentationProvider {
}

private findTarget(node: AstNode, namePath: string): SdsDeclaration | undefined {
let [globalName, ...rest] = this.tokenizeNamepath(namePath);
// `rest` contains pairs of separators and names
if (!globalName || rest.length % 2 !== 0) {
let [globalName, ...rest] = namePath.split('.');
if (!globalName) {
/* c8 ignore next 2 */
return undefined;
}

let current = this.findGlobalDeclaration(node, globalName);

while (current && !isEmpty(rest)) {
const [separator, name] = rest.slice(0, 2);
rest = rest.slice(2);

if (separator === '.') {
current = this.findStaticMember(current, name);
} else if (separator === '#') {
current = this.findInstanceMember(current, name);
} else {
/* c8 ignore next 2 */
return undefined;
}
const name = rest.shift();
current = this.findMember(current, name);
}

return current;
}

private tokenizeNamepath(namePath: string): string[] {
const result = [];
let current = '';

for (const c of namePath) {
if (c === '.' || c === '#') {
result.push(current, c);
current = '';
} else {
current += c;
}
}

result.push(current);
return result;
}

private findGlobalDeclaration(node: AstNode, name: string): SdsDeclaration | undefined {
const description = this.findNameInPrecomputedScopes(node, name) ?? this.findNameInGlobalScope(node, name);
const target = description?.node;
Expand All @@ -240,33 +213,20 @@ export class SafeDsDocumentationProvider extends JSDocDocumentationProvider {
}
}

private findStaticMember(node: SdsDeclaration, name: string | undefined): SdsDeclaration | undefined {
private findMember(node: SdsDeclaration, name: string | undefined): SdsDeclaration | undefined {
if (!name) {
/* c8 ignore next 2 */
return undefined;
}

if (isSdsClass(node)) {
return getClassMembers(node).find((it) => isStatic(it) && it.name === name);
return getClassMembers(node).find((it) => it.name === name);
} else if (isSdsEnum(node)) {
return getEnumVariants(node).find((it) => it.name === name);
} else {
return undefined;
}
}

private findInstanceMember(node: SdsDeclaration, name: string | undefined): SdsDeclaration | undefined {
if (!name) {
/* c8 ignore next 2 */
return undefined;
}

if (isSdsClass(node)) {
return getClassMembers(node).find((it) => !isStatic(it) && it.name === name);
} else {
return undefined;
}
}
}

const isBlockTag = (element: JSDocElement): element is JSDocTag => {
Expand Down
7 changes: 1 addition & 6 deletions packages/safe-ds-lang/src/language/validation/names.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import {
getParameters,
getResults,
getTypeParameters,
isStatic,
streamBlockLambdaResults,
streamPlaceholders,
} from '../helpers/nodeProperties.js';
Expand Down Expand Up @@ -224,11 +223,7 @@ export const classMustContainUniqueNames = (node: SdsClass, accept: ValidationAc
accept,
);

const instanceMembers = getClassMembers(node).filter((it) => !isStatic(it));
namesMustBeUnique(instanceMembers, (name) => `An instance member with name '${name}' exists already.`, accept);

const staticMembers = getClassMembers(node).filter(isStatic);
namesMustBeUnique(staticMembers, (name) => `A static member with name '${name}' exists already.`, accept);
namesMustBeUnique(getClassMembers(node), (name) => `A class member with name '${name}' exists already.`, accept);
};

export const enumMustContainUniqueNames = (node: SdsEnum, accept: ValidationAcceptor): void => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ describe('SafeDsDocumentationProvider', () => {
testName: 'link (instance attribute)',
code: `
/**
* {@link MyClass#myAttribute}
* {@link MyClass.myAttribute}
*/
fun myFunction1()

Expand All @@ -369,7 +369,7 @@ describe('SafeDsDocumentationProvider', () => {
}
`,
predicate: isSdsFunction,
expectedDocumentation: `[MyClass#myAttribute](`,
expectedDocumentation: `[MyClass.myAttribute](`,
},
{
testName: 'link (static attribute)',
Expand All @@ -390,7 +390,7 @@ describe('SafeDsDocumentationProvider', () => {
testName: 'link (instance method)',
code: `
/**
* {@link MyClass#myMethod}
* {@link MyClass.myMethod}
*/
fun myFunction1()

Expand All @@ -399,7 +399,7 @@ describe('SafeDsDocumentationProvider', () => {
}
`,
predicate: isSdsFunction,
expectedDocumentation: `[MyClass#myMethod](`,
expectedDocumentation: `[MyClass.myMethod](`,
},
{
testName: 'link (nested class)',
Expand Down Expand Up @@ -450,7 +450,7 @@ describe('SafeDsDocumentationProvider', () => {
testName: 'link (long chain)',
code: `
/**
* {@link MyClass.NestedClass#myMethod}
* {@link MyClass.NestedClass.myMethod}
*/
fun myFunction1()

Expand All @@ -461,7 +461,7 @@ describe('SafeDsDocumentationProvider', () => {
}
`,
predicate: isSdsFunction,
expectedDocumentation: `[MyClass.NestedClass#myMethod](`,
expectedDocumentation: `[MyClass.NestedClass.myMethod](`,
},
{
testName: 'link (unresolved global)',
Expand All @@ -474,17 +474,6 @@ describe('SafeDsDocumentationProvider', () => {
predicate: isSdsFunction,
expectedDocumentation: `myFunction2`,
},
{
testName: 'link (wrong container for instance)',
code: `
/**
* {@link myFunction1#test}
*/
fun myFunction1()
`,
predicate: isSdsFunction,
expectedDocumentation: `myFunction1#test`,
},
{
testName: 'link (wrong container for static)',
code: `
Expand Down
Loading