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

Classes should be able to access their own fields and methods #17

Merged
merged 2 commits into from
Dec 15, 2020

Conversation

t-8ch
Copy link
Contributor

@t-8ch t-8ch commented Mar 22, 2019

If the author of a class does not want its members not to be accessed by itself, they should not add members in the first place (I would argue).

Usecase:

In jenkins I have an Extension that should look up another extension in its constructor and store it in a field to be used when the callbacks from its ExtensionPoint are executed.

@t-8ch
Copy link
Contributor Author

t-8ch commented Mar 22, 2019

Cc @jglick

@@ -444,7 +455,6 @@ public void visitFieldInsn(int opcode, String owner, String name, String desc) {
}
break;
}
super.visitFieldInsn(opcode, owner, name, desc);
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

@t-8ch t-8ch Apr 1, 2019

Choose a reason for hiding this comment

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

  • It does nothing.
  • super is not used in any of the other override methods.
  • It would have to be duplicated for both method exit points.

@t-8ch
Copy link
Contributor Author

t-8ch commented Apr 2, 2019

@jglick The new version now also handles your example from #6.

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Did you forget to commit the contents of Outer.java?

@t-8ch
Copy link
Contributor Author

t-8ch commented Apr 2, 2019

Indeed. I added it.

@@ -383,24 +383,38 @@ public String getProperty(String key) {
};
}

private static String topLevelClass(String a) {
int i = a.indexOf('$');
Copy link
Member

Choose a reason for hiding this comment

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

By the way this is not technically correct since $ is a legal identifier character, but in practice it is so rarely used that I think this is fine.

jglick added a commit to jglick/jenkins that referenced this pull request Nov 25, 2019
oleg-nenashev pushed a commit to jenkinsci/jenkins that referenced this pull request Jan 21, 2020
* Playing with WebSocket connections.

* Pluggable handlers.

* errorWithoutStack

* Server-side keepalive pings.

* Redesigned to work as an HttpResponse.

* Unused dep.

* Comment on JnlpSlaveRestarterInstaller.

* Sketch of WebSocket-based endpoint for JNLPLauncher.

* Comment.

* Test enhancement.

* Making it more obvious from JNLPLauncherTest where remoting.jar is being loaded from.

* Reworked protocol to negotiate remote capabilities.

* Unhelpful comment.

* Simplifying handshake to use HTTP headers.

* Moving code into top-level classes and otherwise prettifying.

* Linking to upstream PRs.

* Timestamped snapshot + incremental.

* Picking up incrementalified build of winstone.

* Working around jenkinsci/lib-access-modifier#17.

* Finally have an incrementalified build of remoting.

* Use -webSocket option.

* Handling some close and error methods.

* Sending X-Remoting-Minimum-Version.

* If hudson.remoting.jnlp.Main._main fails, show the error.

* Comment.

* GUI analogue of jenkinsci/remoting@86cea5b.

* Missing since tags.

* Capitalization.

* WebSockets.isSupported

* Rather than hiding JNLPLauncher.DescriptorImpl when the TCP port is disabled, always display it, but show form validation appropriate to WebSocket or TCP mode.

* Form validation fixes.

* Minor test improvements.

* Reworked WebSocketAgents to be compatible with JnlpAgentReceiver.

* Removing WebSocketSession.keepAlive in favor of a global setting applicable to all services.

* Add a -webSocket option to the Jenkins CLI.

* Using a snapshot deployment of Remoting, pending INFRA-2379.

* After #3838 there is no reason to recheck authentication after parsing CLICommand arguments.

* Advertise the -webSocket option.

* Adapt to newer HtmlUnit.

* HtmlUnit/htmlunit#29 seems to have been incompatible.
WebClient.addRequestHeader will no longer override a header in a WebRequest,
such as when the same WebClient/WebRequest was previously used with different headers.

* Tracked down a behavioral change in passing through URL-encoded path characters.
HtmlUnit/htmlunit@2c49568 picks up
apache/httpcomponents-client@8c04c6a which is the actual change.

* Disabled TCP port does not matter in WebSocket mode.

* Shade dependencies needed for jenkins-cli.jar.

* Help text edit.

* jenkinsci/winstone#79 was released as 5.5.

* jenkinsci/jenkins-test-harness#183 released as 2.59.

* #4387 (comment)

* Bumping remoting to a new deployed snapshot.

* Need jenkinsci/winstone#86.

* jenkinsci/winstone#86 released as 5.6.

* Introduced constant for X-Remoting-Minimum-Version.

* s/slave/agent/ in GUI

* Removing in-JVM test, as it was no longer useful after introducing shading anyway.

* Bump.

* No need to check for anonymous CONNECT here.

* s/jenkins.slaves/jenkins.agents/g for new code.

* Restrict the diagnostic endpoint to administrators.

* Fixed Javadoc import after package move.

* jenkinsci/remoting#357 released as 4.0.
@timja timja merged commit 32f416c into jenkinsci:master Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants