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 rule to detect redundant // @internal comments #342

Open
ajafff opened this issue Jul 17, 2018 · 6 comments
Open

Add rule to detect redundant // @internal comments #342

ajafff opened this issue Jul 17, 2018 · 6 comments

Comments

@ajafff
Copy link
Member

ajafff commented Jul 17, 2018

// @internal
namespace foo {
  export var v: any;
  // v-- this one is redundant because a parent is already marked as internal
  // @internal
  export var v2: any;
  // v-- this one is redundant as well, just other comment kind
  /* @internal */ export var v3: any;
}

This should probably not be enabled in a preset.

TBD: how should this work if --stripInternal is not enabled?

@ajafff
Copy link
Member Author

ajafff commented Jul 18, 2018

For posterity: TypeScript simply omits any Node that has the substring @internal somewhere in its leasing comments.

I'll do some further investigation to see if there is the need for a rule to disallow @internal comments at invalid/dangerous positions.

@ajafff
Copy link
Member Author

ajafff commented Jul 18, 2018

It should also be an error to have @internal comment in trailing comments

@ajafff
Copy link
Member Author

ajafff commented Jul 18, 2018

Also explore adding errors for internal declarations that are used by other non-internal declarations.

@ajafff
Copy link
Member Author

ajafff commented Jul 18, 2018

The complexity of this topic actually warrants more than one rule: #345 and #346

Regarding // @internal at "dangerous" positions: It turns out you can omit certain certain declarations, but there will be no syntax error:

// emits nothing
export var
  // @internal
  a: any;

// emits 'var b: any'
export var
  b: any,
  // @internal
  c: any;

// doesn't emit the heritage clause, also works for 'implements'
export class Foo
  extends
  // @internal
  Object {}

@ajafff
Copy link
Member Author

ajafff commented Nov 5, 2018

This rule should also detect // @internal comments on local declarations:

// @internal
const foo = 1;

// @internal
export const bar = 1;

expected: error on foo as it's not exported

@ajafff
Copy link
Member Author

ajafff commented Nov 6, 2018

That last thing is actually quite hard to achieve now that declarations are implicitly added to the declaration file. So this needs the same logic as #346

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant