[PATCH v3] userdiff: better method/property matching for C#

From: Steven Jeuris <steven.jeuris@3shape.com>

- Support multi-line methods by not requiring closing parenthesis.
- Support multiple generics (comma was missing before).
- Add missing `foreach`, `lock` and  `fixed` keywords to skip over.
- Remove `instanceof` keyword, which isn't C#.
- Also detect non-method keywords not positioned at the start of a line.
- Added tests; none existed before.

The overall strategy is to focus more on what isn't expected for method/property definitions, instead of what is, but is fully optional.

Signed-off-by: Steven Jeuris <steven.jeuris@gmail.com>
---
    userdiff: better method/property matching for C#
    
    Change since v1: I removed "from" from the list of keywords to skip.
    First, I considered adding "await", but I discovered both "await" and
    "from" are "contextual keywords", which unlike the other keywords
    currently listed, aren't reserved, and can thus cause false negatives.
    I.e., it is valid to have a method named "await" or "from". In edge
    cases, this may lead to false positives, but a different exclusion rule
    will need to be added to handle these.
    
    Change since v2:
    
     * Corrected comment formatting.
     * Added csharp-property-skip-body test.
     * Added comments in test code to explain sections not part of the test.
     * Elaborated regex comments.
     * Excluded math operators (+-*/%) in method pattern to not catch
       multiline operations, and tested for this in the -skip-body tests.
       Catching "-" only worked when it was defined at the end of the
       exclusion block for some reason. The regex matcher seems quite
       bugged.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1682%2FWhathecode%2Fmaster-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1682/Whathecode/master-v3
Pull-Request: https://github.com/git/git/pull/1682

Range-diff vs v2:

 1:  00315519014 ! 1:  9b6c76f5342 userdiff: better method/property matching for C#
     @@ t/t4018/csharp-method (new)
      @@
      +class Example
      +{
     -+	string Method(int RIGHT)
     -+	{
     -+		// Filler
     -+		// Filler
     -+		
     -+		return "ChangeMe";
     -+	}
     ++    string Method(int RIGHT)
     ++    {
     ++        // Filler
     ++        // Filler
     ++
     ++        return "ChangeMe";
     ++    }
      +}
      
       ## t/t4018/csharp-method-explicit (new) ##
     @@ t/t4018/csharp-method-explicit (new)
      +
      +class Example : IDisposable
      +{
     -+	void IDisposable.Dispose() // RIGHT
     -+	{
     -+		// Filler
     -+		// Filler
     -+		
     -+		// ChangeMe
     -+	}
     ++    void IDisposable.Dispose() // RIGHT
     ++    {
     ++        // Filler
     ++        // Filler
     ++        
     ++        // ChangeMe
     ++    }
      +}
      
       ## t/t4018/csharp-method-generics (new) ##
      @@
      +class Example<T1, T2>
      +{
     -+	Example<int, string> Method<TA, TB>(TA RIGHT, TB b)
     -+	{
     -+		// Filler
     -+		// Filler
     -+		
     -+		// ChangeMe
     -+		return null;
     -+	}
     ++    Example<int, string> Method<TA, TB>(TA RIGHT, TB b)
     ++    {
     ++        // Filler
     ++        // Filler
     ++        
     ++        // ChangeMe
     ++        return null;
     ++    }
      +}
      
       ## t/t4018/csharp-method-modifiers (new) ##
     @@ t/t4018/csharp-method-modifiers (new)
      +
      +class Example
      +{
     -+	static internal async Task Method(int RIGHT)
     -+	{
     -+		// Filler
     -+		// Filler
     -+		
     -+		// ChangeMe
     -+		await Task.Delay(1);
     -+	}
     ++    static internal async Task Method(int RIGHT)
     ++    {
     ++        // Filler
     ++        // Filler
     ++        
     ++        // ChangeMe
     ++        await Task.Delay(1);
     ++    }
      +}
      
       ## t/t4018/csharp-method-multiline (new) ##
      @@
      +class Example
      +{
     -+	string Method_RIGHT(
     -+		int a,
     -+		int b,
     -+		int c)
     -+	{
     -+		return "ChangeMe";
     -+	}
     ++    string Method_RIGHT(
     ++        int a,
     ++        int b,
     ++        int c)
     ++    {
     ++        return "ChangeMe";
     ++    }
      +}
      
       ## t/t4018/csharp-method-params (new) ##
      @@
      +class Example
      +{
     -+	string Method(int RIGHT, int b, int c = 42)
     -+	{
     -+		// Filler
     -+		// Filler
     -+		
     -+		return "ChangeMe";
     -+	}
     ++    string Method(int RIGHT, int b, int c = 42)
     ++    {
     ++        // Filler
     ++        // Filler
     ++        
     ++        return "ChangeMe";
     ++    }
      +}
      
       ## t/t4018/csharp-method-skip-body (new) ##
     @@ t/t4018/csharp-method-skip-body (new)
      +
      +class Example : IDisposable
      +{
     -+	string Method(int RIGHT)
     -+	{
     -+		// Method calls
     -+		MethodCall();
     -+		MethodCall(1, 2);
     -+		MethodCall(
     -+			1, 2);
     -+		
     -+		// Assignments
     -+		var constantAssignment = "test";
     -+		var methodAssignment = MethodCall();
     -+		var multiLineMethodAssignment = MethodCall(
     -+			);
     -+		
     -+		// Initializations/disposal
     -+		new Example();
     -+		new Example(
     -+			);
     -+		new Example { };
     -+		using (this) 
     -+		{
     -+		}
     -+		var def =
     -+			this is default(
     -+				Example);
     -+		
     -+		// Iteration statements
     -+		do { } while (true);
     -+		do MethodCall(
     -+			); while (true);
     -+		while (true);
     -+		while (true) {
     -+			break;
     -+		}
     -+		for (int i = 0; i < 10; ++i)
     -+		{
     -+		}
     -+		foreach (int i in Enumerable.Range(0, 10))
     -+		{
     -+		}
     -+		int[] numbers = [5, 4, 1, 3, 9, 8, 6, 7, 2, 0];
     -+		
     -+		// Control
     -+		if (false)
     -+		{
     -+			return "out";
     -+		}
     -+		else { }
     -+		if (true) MethodCall(
     -+			);
     -+		else MethodCall(
     -+			);
     -+		switch ("test")
     -+		{
     -+			case "one":
     -+				return MethodCall(
     -+					);
     -+			case "two":
     -+				break;
     -+		}
     -+		(int, int) tuple = (1, 4);
     -+		switch (tuple)
     -+		{
     -+			case (1, 4):
     -+				MethodCall();
     -+		}
     -+		
     -+		// Exceptions
     -+		try
     -+		{
     -+			throw new Exception("fail");
     -+		}
     -+		catch (Exception)
     -+		{
     -+		}
     -+		finally
     -+		{
     -+		}
     -+		try { } catch (Exception) {}
     -+		try
     -+		{
     -+			throw GetException(
     -+				);
     -+		}
     -+		catch (Exception) { }
     -+		
     -+		// Others
     -+		lock (this)
     -+		{
     -+		}
     -+		unsafe
     -+		{
     -+			byte[] bytes = [1, 2, 3];
     -+			fixed (byte* pointerToFirst = bytes)
     -+			{
     -+			}
     -+		}
     -+		
     -+		return "ChangeMe";
     -+	}
     -+	
     -+	public void Dispose() {}
     -+	
     -+	string MethodCall(int a = 0, int b = 0) => "test";
     -+	Exception GetException() => new Exception("fail");
     -+	int[] Numbers() => [0, 1];
     ++    string Method(int RIGHT)
     ++    {
     ++        // Method calls
     ++        MethodCall();
     ++        MethodCall(1, 2);
     ++        MethodCall(
     ++            1, 2);
     ++
     ++        // Assignments
     ++        var constantAssignment = "test";
     ++        var methodAssignment = MethodCall();
     ++        var multiLineMethodAssignment = MethodCall(
     ++        );
     ++        var multiLine = "first"
     ++            + MethodCall()
     ++            - MethodCall()
     ++            + MethodCall();
     ++
     ++        // Initializations/disposal
     ++        new Example();
     ++        new Example(
     ++            );
     ++        new Example { };
     ++        using (this)
     ++        {
     ++        }
     ++        var def =
     ++            this is default(
     ++                Example);
     ++
     ++        // Iteration statements
     ++        do { } while (true);
     ++        do MethodCall(
     ++        ); while (true);
     ++        while (true);
     ++        while (true) {
     ++            break;
     ++        }
     ++        for (int i = 0; i < 10; ++i)
     ++        {
     ++        }
     ++        foreach (int i in Enumerable.Range(0, 10))
     ++        {
     ++        }
     ++        int[] numbers = [5, 4, 1, 3, 9, 8, 6, 7, 2, 0];
     ++
     ++        // Control
     ++        if (false)
     ++        {
     ++            return "out";
     ++        }
     ++        else { }
     ++        if (true) MethodCall(
     ++        );
     ++        else MethodCall(
     ++        );
     ++        switch ("test")
     ++        {
     ++            case "one":
     ++            return MethodCall(
     ++            );
     ++            case "two":
     ++            break;
     ++        }
     ++        (int, int) tuple = (1, 4);
     ++        switch (tuple)
     ++        {
     ++            case (1, 4):
     ++            MethodCall();
     ++        }
     ++
     ++        // Exceptions
     ++        try
     ++        {
     ++            throw new Exception("fail");
     ++        }
     ++        catch (Exception)
     ++        {
     ++        }
     ++        finally
     ++        {
     ++        }
     ++        try { } catch (Exception) {}
     ++        try
     ++        {
     ++            throw GetException(
     ++            );
     ++        }
     ++        catch (Exception) { }
     ++
     ++        // Others
     ++        lock (this)
     ++        {
     ++        }
     ++        unsafe
     ++        {
     ++            byte[] bytes = [1, 2, 3];
     ++            fixed (byte* pointerToFirst = bytes)
     ++            {
     ++            }
     ++        }
     ++
     ++        return "ChangeMe";
     ++    }
     ++
     ++    // Supporting methods to make the tested method above valid C#.
     ++    public void Dispose() {}
     ++    string MethodCall(int a = 0, int b = 0) => "test";
     ++    Exception GetException() => new Exception("fail");
     ++    int[] Numbers() => [0, 1];
      +}
      
       ## t/t4018/csharp-method-special-chars (new) ##
      @@
      +class @Some_Type
      +{
     -+	@Some_Type @Method_With_Underscore(int RIGHT)
     -+	{
     -+		// Filler
     -+		// Filler
     -+		
     -+		// ChangeMe
     -+		return new @Some_Type();
     -+	}
     ++    @Some_Type @Method_With_Underscore(int RIGHT)
     ++    {
     ++        // Filler
     ++        // Filler
     ++        
     ++        // ChangeMe
     ++        return new @Some_Type();
     ++    }
      +}
      
       ## t/t4018/csharp-method-with-spacing (new) ##
     @@ t/t4018/csharp-method-with-spacing (new)
      +{
      +		string   Method 	( int 	RIGHT )
      +	{
     -+		// Filler
     -+		// Filler
     ++        // Filler
     ++        // Filler
      +		
     -+		return "ChangeMe";
     -+	}
     ++        return "ChangeMe";
     ++    }
      +}
      
       ## t/t4018/csharp-property (new) ##
      @@
      +class Example
      +{
     -+	public bool RIGHT
     ++    public bool RIGHT
      +    {
      +        get { return true; }
      +        set
     @@ t/t4018/csharp-property (new)
      +            // ChangeMe
      +        }
      +    }
     ++}
     +
     + ## t/t4018/csharp-property-skip-body (new) ##
     +@@
     ++using System.Linq;
     ++using System;
     ++
     ++class Example : IDisposable
     ++{
     ++    public string RIGHT
     ++    {
     ++        get
     ++        {
     ++            // Method calls
     ++            MethodCall();
     ++            MethodCall(1, 2);
     ++            MethodCall(
     ++                1, 2);
     ++
     ++            // Assignments
     ++            var constantAssignment = "test";
     ++            var methodAssignment = MethodCall();
     ++            var multiLineMethodAssignment = MethodCall(
     ++                );
     ++            var multiLine = "first"
     ++                + MethodCall()
     ++                - MethodCall()
     ++                + MethodCall();
     ++
     ++            // Initializations/disposal
     ++            new Example();
     ++            new Example(
     ++                );
     ++            new Example { };
     ++            using (this)
     ++            {
     ++            }
     ++            var def =
     ++                this is default(
     ++                    Example);
     ++
     ++            // Iteration statements
     ++            do { } while (true);
     ++            do MethodCall(
     ++            ); while (true);
     ++            while (true);
     ++            while (true) {
     ++                break;
     ++            }
     ++            for (int i = 0; i < 10; ++i)
     ++            {
     ++            }
     ++            foreach (int i in Enumerable.Range(0, 10))
     ++            {
     ++            }
     ++            int[] numbers = [5, 4, 1, 3, 9, 8, 6, 7, 2, 0];
     ++
     ++            // Control
     ++            if (false)
     ++            {
     ++                return "out";
     ++            }
     ++            else { }
     ++            if (true) MethodCall(
     ++            );
     ++            else MethodCall(
     ++            );
     ++            switch ("test")
     ++            {
     ++                case "one":
     ++                return MethodCall(
     ++                );
     ++                case "two":
     ++                break;
     ++            }
     ++            (int, int) tuple = (1, 4);
     ++            switch (tuple)
     ++            {
     ++                case (1, 4):
     ++                MethodCall();
     ++            }
     ++
     ++            // Exceptions
     ++            try
     ++            {
     ++                throw new Exception("fail");
     ++            }
     ++            catch (Exception)
     ++            {
     ++            }
     ++            finally
     ++            {
     ++            }
     ++            try { } catch (Exception) {}
     ++            try
     ++            {
     ++                throw GetException(
     ++                );
     ++            }
     ++            catch (Exception) { }
     ++
     ++            // Others
     ++            lock (this)
     ++            {
     ++            }
     ++            unsafe
     ++            {
     ++                byte[] bytes = [1, 2, 3];
     ++                fixed (byte* pointerToFirst = bytes)
     ++                {
     ++                }
     ++            }
     ++
     ++            return "ChangeMe";
     ++        }
     ++        set { }
     ++    }
     ++
     ++    // Supporting methods to make the tested property above valid C#.
     ++    public void Dispose() {}
     ++    string MethodCall(int a = 0, int b = 0) => "test";
     ++    Exception GetException() => new Exception("fail");
     ++    int[] Numbers() => [0, 1];
      +}
      
       ## userdiff.c ##
     @@ userdiff.c: PATTERNS("cpp",
      -	 "!^[ \t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\n"
      -	 /* Methods and constructors */
      -	 "^[ \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe|async)[ \t]+)*[][<>@.~_[:alnum:]]+[ \t]+[<>@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n"
     +-	 /* Properties */
     +-	 "^[ \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \t]+)*[][<>@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ \t]*$\n"
      +	 /*
     -+	  * Jump over keywords not used by methods which can be followed by parentheses without special characters in between,
     ++	  * Jump over reserved keywords which are illegal method names, but which
     ++	  * can be followed by parentheses without special characters in between,
      +	  * making them look like methods.
      +	  */
      +	 "!(^|[ \t]+)(do|while|for|foreach|if|else|new|default|return|switch|case|throw|catch|using|lock|fixed)([ \t(]+|$)\n"
     -+	 /* Methods/constructors:
     -+	  * the strategy is to identify a minimum of two groups (any combination of keywords/type/name),
     -+	  * without intermediate or final characters which can't be part of method definitions before the opening parenthesis.
     ++	 /*
     ++	  * Methods/constructors:
     ++	  * The strategy is to identify a minimum of two groups (any combination
     ++	  * of keywords/type/name), without intermediate characters which can't be
     ++	  * part of method definitions before the opening parenthesis, and without
     ++	  * final unexpected characters, normally only used in ordinary statements.
     ++	  * "=" is excluded to ignore assignments, but as a result rules out
     ++	  * methods with expression bodies. However, since those fit on 1-2 lines,
     ++	  * a chunk header isn't useful either way.
      +	  */
     -+	 "^[ \t]*(([][[:alnum:]@_<>.,]*[^=:{ \t][ \t]+[][[:alnum:]@_<>.,]*)+\\([^;]*)$\n"
     - 	 /* Properties */
     --	 "^[ \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \t]+)*[][<>@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ \t]*$\n"
     -+	 "^[ \t]*((([][[:alnum:]@_<>.,]+)[ \t]+[][[:alnum:]@_]*)+[^=:;,()]*)$\n"
     ++	 "^[ \t]*(([][[:alnum:]@_<>.,]*[^=:{ \t+*%\\/\\-][ \t]+[][[:alnum:]@_<>.,]*)+\\([^;]*)$\n"
     ++	 /*
     ++	  * Properties:
     ++	  * As with methods, expect a minimum of two groups. And, the vast majority
     ++	  * of properties long enough to be worth showing a chunk header for don't
     ++	  * include "=:;,()" on the line they are defined.
     ++	  */
     ++	 "^[ \t]*([][[:alnum:]@_<>.,]+[ \t]+[][[:alnum:]@_.]+[^=:;,()]*)$\n"
       	 /* Type definitions */
       	 "^[ \t]*(((static|public|internal|private|protected|new|unsafe|sealed|abstract|partial)[ \t]+)*(class|enum|interface|struct|record)[ \t]+.*)$\n"
       	 /* Namespace */

 t/t4018/csharp-method               |  10 +++
 t/t4018/csharp-method-explicit      |  12 +++
 t/t4018/csharp-method-generics      |  11 +++
 t/t4018/csharp-method-modifiers     |  13 +++
 t/t4018/csharp-method-multiline     |  10 +++
 t/t4018/csharp-method-params        |  10 +++
 t/t4018/csharp-method-skip-body     | 116 +++++++++++++++++++++++++++
 t/t4018/csharp-method-special-chars |  11 +++
 t/t4018/csharp-method-with-spacing  |  10 +++
 t/t4018/csharp-property             |  11 +++
 t/t4018/csharp-property-skip-body   | 120 ++++++++++++++++++++++++++++
 userdiff.c                          |  30 +++++--
 12 files changed, 358 insertions(+), 6 deletions(-)
 create mode 100644 t/t4018/csharp-method
 create mode 100644 t/t4018/csharp-method-explicit
 create mode 100644 t/t4018/csharp-method-generics
 create mode 100644 t/t4018/csharp-method-modifiers
 create mode 100644 t/t4018/csharp-method-multiline
 create mode 100644 t/t4018/csharp-method-params
 create mode 100644 t/t4018/csharp-method-skip-body
 create mode 100644 t/t4018/csharp-method-special-chars
 create mode 100644 t/t4018/csharp-method-with-spacing
 create mode 100644 t/t4018/csharp-property
 create mode 100644 t/t4018/csharp-property-skip-body