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

Feature ataraxie/codeshovel#64: Extend CodeShovel to capture JavaDoc #65

Merged

Conversation

ishtiaque05
Copy link
Collaborator

  • Add a json key functionJavaDoc which contains the java doc string
    TODO: implement "getInitialJavaDoc" for python and typescript
  • For python implement here: PythonFunction.java
  • For typescript implement here: TypeScriptFunction.java
    Discussion link: Track method documentation changes #64

- Add a json key `functionJavaDoc` which contains the java doc string
TODO: implement "getInitialJavaDoc" for python and typescript
- For python implement here: PythonFunction.java
- For typescript implement here: TypeScriptFunction.java
Discussion link: ataraxie#64
@ishtiaque05 ishtiaque05 marked this pull request as draft February 23, 2021 21:11
@ishtiaque05
Copy link
Collaborator Author

I realized getInitialJavaDoc is a poor choice of name and should be change to getInitialDoc

- Add a new change type Ydocchange which extends Ysignaturechange
- Rename everywhere reference of *javaDoc* -> Doc to keep things consistent with python
  and typescript implementation
TODO list:
- Implement `getInitialDoc` in `com/felixgrund/codeshovel/parser/impl/PythonFunction.java`
- Implement `getInitialDoc` in `com/felixgrund/codeshovel/parser/impl/TypeScriptFunction.java`
- Handle `Ydocchange` in `src/main/java/com/felixgrund/codeshovel/parser/impl/TypeScriptParser.java`
- Handle `Ydocchange` in `src/main/java/com/felixgrund/codeshovel/parser/impl/PythonParser.java`
- Get the diff for doc changes
@ishtiaque05
Copy link
Collaborator Author

ishtiaque05 commented Feb 23, 2021

Feature #64

  • Add a new change type Ydocchange which extends Ysignaturechange
  • Rename everywhere reference of javaDoc -> Doc to keep things consistent with python
    and typescript implementation
    TODO list:
  • Implement getInitialDoc in com/felixgrund/codeshovel/parser/impl/PythonFunction.java
  • Implement getInitialDoc in com/felixgrund/codeshovel/parser/impl/TypeScriptFunction.java
  • Handle Ydocchange in src/main/java/com/felixgrund/codeshovel/parser/impl/TypeScriptParser.java
  • Handle Ydocchange in src/main/java/com/felixgrund/codeshovel/parser/impl/PythonParser.java
  • Implement Ydocchange for typescript and python #69
  • Get the diff for doc changes
  • Run mvn test and fix some test

An example of output for reference:

"f7cda475cd784369a345bcee687bb2de630c8d6a": {
      "type": "Ydocchange",
      "commitMessage": "joc change\n",
      "commitDate": "23/02/21 1:39 PM",
      "commitName": "f7cda475cd784369a345bcee687bb2de630c8d6a",
      "commitAuthor": "Syed Ishtiaque Ahmad",
      "commitDateOld": "23/02/21 12:21 PM",
      "commitNameOld": "76efe8b277f2be70003630ef53b4b44ebc123ec2",
      "commitAuthorOld": "Syed Ishtiaque Ahmad",
      "daysBetweenCommits": 0.05,
      "commitsBetweenForRepo": 1,
      "commitsBetweenForFile": 1,
      "actualSource": "@SuppressWarnings(\"Hello changed the annotation\")\npublic void testSumagain() throws ArithmeticException, ArrayIndexOutOfBoundsExceptionthrows {\n    Calculator calculator \u003d new Calculator();\n    int result \u003d calculator.sum(2, 2);\n    int iby4 \u003d calculator.incrementBy4(result);\n    Assert.assertTrue(result \u003d\u003d 4);\n    Assert.assertEquals(8, iby4);\n}",
      "path": "src/test/java/com/github/stokito/unitTestExample/calculator/CalculatorTest.java",
      "functionStartLine": 14,
      "functionName": "testSumagain",
      "functionAnnotation": "@SuppressWarnings(\"Hello changed the annotation\")",
      "functionDoc": "\n@params empty params\n@throws ArithmeticException\n@throws ArrayIndexOutOfBoundsExceptionthrows\n",
      "diff": "",
      "extendedDetails": {
        "oldValue": "\n@throws ArithmeticException\n@throws ArrayIndexOutOfBoundsExceptionthrows\n",
        "newValue": "\n@params empty params\n@throws ArithmeticException\n@throws ArrayIndexOutOfBoundsExceptionthrows\n"
      }
    }

@braxtonhall
Copy link
Collaborator

I'm realizing that this may be a bigger change than the annotation change. We would probably want to use javadoc in the similarity algorithm.

@ishtiaque05
Copy link
Collaborator Author

ishtiaque05 commented Feb 24, 2021

I'm realizing that this may be a bigger change than the annotation change. We would probably want to use javadoc in the similarity algorithm.

Is this because you want to track JavadocBlockTag such as PARAM, RETURN, THROWS etc changes?

Example of a proper javadoc with all its element block tag:

/**
	 * Subject line
	 *
	 * <p>Description of the method with optional {@code code} and {@link Object links to Javadoc}
	 * </p>
	 *
	 * <pre>
	 *    raw input
	 * </pre>
	 *
	 * @param foo first arg
	 * @return a bar
	 * @throws SomeException if bar goes wrong
	 * @see someOtherMethod()
	 * @since 2.2.2
	 * @author me
*/

@ishtiaque05
Copy link
Collaborator Author

Finally, I was able to run test locally 🎉 and now we have Ydocchange test failures. I will fix them and push it 😃

Ydocchange

@braxtonhall
Copy link
Collaborator

I'm realizing that this may be a bigger change than the annotation change. We would probably want to use javadoc in the similarity algorithm.

Is this because you want to track JavadocBlockTag such as PARAM, RETURN, THROWS etc changes?

Example of a proper javadoc with all its element block tag:

/**
	 * Subject line
	 *
	 * <p>Description of the method with optional {@code code} and {@link Object links to Javadoc}
	 * </p>
	 *
	 * <pre>
	 *    raw input
	 * </pre>
	 *
	 * @param foo first arg
	 * @return a bar
	 * @throws SomeException if bar goes wrong
	 * @see someOtherMethod()
	 * @since 2.2.2
	 * @author me
*/

Sorry should have replied here 😅

I think the description of the method may be useful in discerning if two methods match or are "the same method".
It seems like it would be wasteful to not account for the Javadoc in the similarity calculation

@ishtiaque05 ishtiaque05 force-pushed the feature/track-method-documentation branch from b8b1a6e to 09ce0d3 Compare March 7, 2021 07:16
@ishtiaque05 ishtiaque05 marked this pull request as ready for review March 8, 2021 20:24
@ishtiaque05 ishtiaque05 marked this pull request as draft March 8, 2021 20:25
@ishtiaque05 ishtiaque05 requested a review from braxtonhall March 12, 2021 01:05
@rtholmes
Copy link
Collaborator

rtholmes commented Mar 12, 2021

@ishtiaque05 great work with this PR. It's really interesting to go through the oracles to see how pervasive doc changes are (and how they do (and do not) line up with other kinds of changes.

@ishtiaque05 ishtiaque05 requested a review from rtholmes March 12, 2021 21:10
@ishtiaque05 ishtiaque05 marked this pull request as ready for review March 12, 2021 21:10
@ishtiaque05
Copy link
Collaborator Author

ishtiaque05 commented Mar 12, 2021

Hi, could we merge this with the develop branch and work on Javadoc as a part of the similarity algorithm and getting the diff in a separate issue, please?

Copy link
Collaborator

@rtholmes rtholmes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

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.

3 participants