Add check for redundant attributes (for now: redundant access specifier)

[wilzbach]

When I started to write this check, it was mainly to avoid redundant access specifiers like:

private:
   private int foo;

However, I realized that it can work with any kind of attribute, e.g.

@safe:
   @safe void foo() {}

or

@myCustomUDA {
    @myCustomUDA void foo() {}
} 

However, for now I lack the time to properly test all attribute combination, so I wrote generic code, but only enabled it for access specifier. It shouldn't be too hard to extend it to all attributes later.

[ghost]

I'd like to see a test for

private:
public int a;
private: // should warn here

[ghost]

style + use same name as in the grammar

[ghost]

"access specifier" is inaccurate, it's called a visibility attribute.
If you add support for other attributes you'll have to think to use the correct term too.

[ghost]

please don't blend the brace styles. same remark plenty of times !

[ghost]

small remark for the future: you can push the feature branch directly on dlang-community. It's easier for the reviewers to pull the branch on their clone.

[wilzbach]

I'd like to see a test for

Added. Btw dlang/phobos#5477 contains all found instances in Phobos, but as mentioned it doesn't check all attributes (for now).

style + use same name as in the grammar

Fixed. Sorry.

small remark for the future: you can push the feature branch directly on dlang-community. It's easier for the reviewers to pull the branch on their clone.

Okay. I will do so next time.
Btw you can rather easily pull a PR with this git config:

[remote "upstream"]
	fetch = +refs/pull/*/head:refs/remotes/upstream/pr/*

(or master if you don't use an upstream here).

and then e.g. git checkout pr/441

[ghost]

I suspected the current failure. It's on the test i asked you to add.

[wilzbach]

I suspected the current failure. It's on the test i asked you to add.

Hmm are you sure?
AFAICT I just forgot to adjust the line numbers after changing the brace style?

[ghost]

AFAICT I just forgot to adjust the line numbers after changing the brace style?

Right.

Appveyor icon is still there ?

[wilzbach]

Appveyor icon is still there ?

I think GH isn't that smart - it will just be gone for new PRs ...

[ghost]

looks good now.

[wilzbach]

(rebased to master)

[ghost]

Do you want to use this on phobos ASAP ? For me it's quite mergeable in the current state. Also you didn't touch it last 2 days, which means that it's also okay for you.

[wilzbach]

Do you want to use this on phobos ASAP ?

I currently have other goals, but eventually at some point in the future, yes.
But I haven't had time to fix dlang/phobos#5477, so there's no need to hurry.
I just saw the merge error here ...

For me it's quite mergeable in the current state. Also you didn't touch it last 2 days, which means that it's also okay for you.

I eventually want to make it work with all kind of attributes, but unfortunately not in this month or in the foreseeable future.

[ghost]

I eventually want to make it work with all kind of attributes

There's chance that the same check for all the attributes require much more work and will not be compatible with the current structure. IIRC for a @safe struct{} safety is propagated to functions templates too, so processing is different.