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

Remove empty attribute parenthesis and attribute suffix if exists #118

Merged
merged 2 commits into from Nov 5, 2019

Conversation

danielmarbach
Copy link
Member

@danielmarbach danielmarbach commented Nov 2, 2019

fixes #105

@danielmarbach
Copy link
Member Author

danielmarbach commented Nov 2, 2019 via email

@danielmarbach
Copy link
Member Author

danielmarbach commented Nov 2, 2019 via email

@jnm2
Copy link
Contributor

jnm2 commented Nov 2, 2019

This one is going to be hard to explain, but it works in each combination of cases:

from input in new[]
{
    "[FooAttribute]",
    "[FooAttribute()]",
    "[FooAttribute(2)]",
    "[Foo]",
    "[Foo()]",
    "[Foo(2)]"
}
select Regex.Replace(input, @"(?<=\[.*)(Attribute)?(\(\)(?=\])|(?=\([^)]+\)\]))?", string.Empty)

Output:

[Foo]
[Foo]
[Foo(2)]
[Foo]
[Foo]
[Foo(2)]

@sungam3r
Copy link
Member

sungam3r commented Nov 2, 2019

Regular expressions have always been some sort magic to me.

@jnm2
Copy link
Contributor

jnm2 commented Nov 2, 2019

It's bad. I documented it and made some changes that seemed important while writing comments:

from input in new[]
{
    "[FooAttribute]",
    "[FooAttribute()]",
    "[FooAttribute(2)]",
    "[Foo]",
    "[Foo()]",
    "[Foo(2)]"
}
select Regex.Replace(
    input,
    @"
        (?<=\[[^\]]*)     # Require a starting square brace on the same line and any following characters except an ending square brace.
        (Attribute)?      # Delete this if present.
        (                 # Then require either:
            (\(\))?(?=\]) # - Empty parens (to delete) if present, immediately followed by closing square brace (not deleted),
        |
            (?=\(.+\)\])  # - or non-empty parens immediately followed by closing square brace (not deleted).
        )",
    string.Empty,         // (Replacing with empty string is referred to above as deleting.)
    RegexOptions.IgnorePatternWhitespace)

@jnm2
Copy link
Contributor

jnm2 commented Nov 2, 2019

It's still pretty bad. Don't feel like you have to do anything with it. :D

@danielmarbach danielmarbach changed the title Remove empty attribute parenthesis Remove empty attribute parenthesis and attribute suffix if exists Nov 5, 2019
@danielmarbach
Copy link
Member Author

@sungam3r @jnm2 have a look. I think this should cover all the cases

@sungam3r
Copy link
Member

sungam3r commented Nov 5, 2019

What about this?

 public class Attribute1 : Attribute
 {
 }

 public class Attribute1Attribute : Attribute
 {
 }

 [@Attribute1Attribute]
 [@Attribute1]
 public class Class1
 {
 }

@danielmarbach
Copy link
Member Author

Honestly, I have no good idea for now how to solve

#118 (comment)

other than tracking all the attributes generated and then find out if a potential clash could be produced and then special name again. Which to me seems super silly for such an edge case.

And making everything pluggable and configurable seems overkill as well. Do you guys see this special case as a killer for this feature or should I proceed?

@jnm2
Copy link
Contributor

jnm2 commented Nov 5, 2019

Nope, I really don't think that situation will really happen in real-world projects. If it does, it won't be hard for the users to figure out what happened since their C# source code that they'll immediately compare with will look so unusual in the first place.

@sungam3r
Copy link
Member

sungam3r commented Nov 5, 2019

Nope, but let's leave a note about this special case near to the regex for attributes in CodeNormalizer.

Cleanup

Update src/PublicApiGenerator/CodeNormalizer.cs

Co-Authored-By: Joseph Musser <me@jnm2.com>

Explanation
@danielmarbach danielmarbach merged commit f6971cd into master Nov 5, 2019
@danielmarbach danielmarbach deleted the empty-attribute-paranthesis branch November 5, 2019 22:58
@danielmarbach danielmarbach added this to the 10.0.0 milestone Nov 27, 2019
@danielmarbach danielmarbach mentioned this pull request Nov 27, 2019
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Removing empty attribute parentheses and Attribute suffix if present
3 participants