{"action":"created","issue":{"url":"https://api.github.com/repos/checkstyle/checkstyle/issues/2852","labels_url":"https://api.github.com/repos/checkstyle/checkstyle/issues/2852/labels{/name}","comments_url":"https://api.github.com/repos/checkstyle/checkstyle/issues/2852/comments","events_url":"https://api.github.com/repos/checkstyle/checkstyle/issues/2852/events","html_url":"https://github.com/checkstyle/checkstyle/pull/2852","id":129177904,"number":2852,"title":"Issue #2822: Makes FinalClass doesnt recognise as final classes with …","user":{"login":"liscju","id":6627878,"avatar_url":"https://avatars.githubusercontent.com/u/6627878?v=3","gravatar_id":"","url":"https://api.github.com/users/liscju","html_url":"https://github.com/liscju","followers_url":"https://api.github.com/users/liscju/followers","following_url":"https://api.github.com/users/liscju/following{/other_user}","gists_url":"https://api.github.com/users/liscju/gists{/gist_id}","starred_url":"https://api.github.com/users/liscju/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/liscju/subscriptions","organizations_url":"https://api.github.com/users/liscju/orgs","repos_url":"https://api.github.com/users/liscju/repos","events_url":"https://api.github.com/users/liscju/events{/privacy}","received_events_url":"https://api.github.com/users/liscju/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2016-01-27T15:31:12Z","updated_at":"2016-01-30T13:54:43Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/checkstyle/checkstyle/pulls/2852","html_url":"https://github.com/checkstyle/checkstyle/pull/2852","diff_url":"https://github.com/checkstyle/checkstyle/pull/2852.diff","patch_url":"https://github.com/checkstyle/checkstyle/pull/2852.patch"},"body":"…nested subclasses\r\n\r\nIn current implementation there is a problem that class with private constructor cannot be made final if there is an inner class that extends it.\r\n\r\nBiggest problem with implementation deals with the fact that extends clause can contain class name, or class name with package for example:\r\n```java\r\nprivate class ExtendA extends A {}\r\nprivate class ExtendC extends com.nonexistent.packages.C {}\r\n```\r\n\r\nTo deal with this fact I put full class name(with package information) to ClassDesc, later when it decides if there is an inner class extending outer, we check if in extends clause there is only class name or there is package information and do appropriate comparison with outer classes names.\r\n"},"comment":{"url":"https://api.github.com/repos/checkstyle/checkstyle/issues/comments/177183458","html_url":"https://github.com/checkstyle/checkstyle/pull/2852#issuecomment-177183458","issue_url":"https://api.github.com/repos/checkstyle/checkstyle/issues/2852","id":177183458,"user":{"login":"romani","id":812984,"avatar_url":"https://avatars.githubusercontent.com/u/812984?v=3","gravatar_id":"","url":"https://api.github.com/users/romani","html_url":"https://github.com/romani","followers_url":"https://api.github.com/users/romani/followers","following_url":"https://api.github.com/users/romani/following{/other_user}","gists_url":"https://api.github.com/users/romani/gists{/gist_id}","starred_url":"https://api.github.com/users/romani/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/romani/subscriptions","organizations_url":"https://api.github.com/users/romani/orgs","repos_url":"https://api.github.com/users/romani/repos","events_url":"https://api.github.com/users/romani/events{/privacy}","received_events_url":"https://api.github.com/users/romani/received_events","type":"User","site_admin":false},"created_at":"2016-01-30T13:54:43Z","updated_at":"2016-01-30T13:54:43Z","body":"some items are not related to your , but please fix them while I see that as a problem\r\n\r\n1)\r\n> `private final Deque<ClassDesc> classes = new ArrayDeque<>();`\r\n\r\nplease do clean of this field in method beginTree that need to be overridden. Instance of Check is reused between all java files, so all fields that store state - need to be cleaned. In Other case there could be interesting and hard to reproduce issues.\r\n\r\n2)\r\n>             final boolean isFinal = modifiers.branchContains(TokenTypes.FINAL);\r\n            final boolean isAbstract = modifiers.branchContains(TokenTypes.ABSTRACT);\r\n\r\nplease move this variables close to usages, we have special Check for this, but it skips \"final\" variables for good reason.\r\n\r\n3)\r\n> final String packageName = getPackageName(ast);\r\n\r\ndo traversing of tree is not good, especially is Check is already not a state-less (have non configuration fields).\r\nPlease extend AcceptedTokens to contain PACKAGE token. visitToken will be called in right place and you will easily collect package name.\r\n\r\n4)\r\n> outerClassFullName\r\n\r\nLets use JDK specification terms, https://docs.oracle.com/javase/specs/jls/se8/html/jls-6.html#d5e10153 . So please rename all \"FullNames\" to \"QualifiedName\".\r\n\r\n5)\r\n> reportNestedSubclass\r\n\r\nI do not like prefix in name \"report.....\" please rename it to \"register....\". Please update all other methods too to keep code consistent.\r\n\r\n6)\r\n> reportNestedSubclass\r\n\r\nI do not like this method, I do believe that logic will be more clear if you embed it to visitToken and move calculation of superClass name to a function.\r\n"}}